dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.21k stars 9.95k forks source link

Clarify intentions for dynamic preventDefault #48193

Closed SteveSandersonMS closed 1 year ago

SteveSandersonMS commented 1 year ago

In https://github.com/dotnet/AspNetCore.Docs/issues/28692, @LunicLynx pointed out a behavior that I did not realise we had. In WebAssembly, it's possible to vary a @onsomeevent:preventDefault flag while that event handler is running, and it happens to produce exactly the behavior people would want.

For example, you can dynamically block specific keystrokes using only C# (previously we have told people to add a JS event handler for this): https://blazorrepl.telerik.com/GduTFbPQ31hhoo2926

Given how this looks like an intentional framework feature, people are likely to use and rely on it. So,

Note that this functionality is WebAssembly-only. We don't mind having WebAssembly-only features, but we prefer it when the failure mode is clearer. For example, giving a compile-time or runtime error if it is used in an unsupported mode. Unfortunately in this case the behavior if you run the same component on Blazor Server really bad. It won't block the keystroke you think it's going to block, but then it does block the next keystroke. This would be a bizarre bug in people's apps.

javiercn commented 1 year ago

My take would be that we don't retain this functionality, it will only work in webassembly and potentially preclude/coplicate the implementation if in the future we want to move the .NET runtime to run in a worker thread. But mostly, it doesn't work on server, so it produces a subpar experience if people start relying on it.

LunicLynx commented 1 year ago

I would argue, that a solution to this would also be good for Server mode. This is a common use case and really limits what can be done without JavaScript.

And strongly against taking it away, making the webassembly mode even less capable 😕

javiercn commented 1 year ago

@LunicLynx it doesn't work in server mode. This relies on the communication being synchronous, which is only possible in webassembly, and just because we run on the main thread, which we might not want to do for performance reasons.

LunicLynx commented 1 year ago

I know, but I think finding a solution is possible. For example executing the synchronous part in webassembly and only the rest in the server would be an idea that could work.

Not saying this is how todo it, just that it is not impossible. It should have the same limitations as JavaScript. As long as the callback is synchronous, this works.

Or don’t, just don’t start eliminating features that should be there just because they don’t fit another mode. That would be really frustrating.

javiercn commented 1 year ago

@LunicLynx I think you are missing context on how server or mobile works. There is no "synchronous" part. You don't get to run any c# code synchronously.

Or don’t, just don’t start eliminating features that should be there just because they don’t fit another mode. That would be really frustrating.

This is not a feature, it's undefined behavior. We don't document it and we don't claim to support it. It's a consequence of the current webassembly implementation that happens to work today, but that we don't make any guarantees will continue to work.

LunicLynx commented 1 year ago

Ok, I really don't like how this is going. (It could be that I don't miss anything here, just saying.)

I want to understand what my expectations of Blazor can be. As I took it, Blazor is considered to be a SPA framework that can replace angular or react (for example).

Even taking the main statement from the Blazor landing page I have a certain expectation.

image

At this point when I can't conditionally limit the propagation or default behavior of an event, I have to ask what is Blazor supposed to be?

Developing custom controls with Blazor for Blazor is not the Idea?

The example I created, which Steve posted is a bit stupid, I guess, and does not display the usefulness of this feature. Creating a full featured dropdown control in just .NET code, requires logic like this as well. (arrow keys, enter)

If the answer is: "Blazor is not for the creation of custom controls, you need to do that in JS." then I can live with that, but this wasn't my understanding, as this can be done by other SPA Frameworks.

I'm really sorry for bothering you about this, but I put some extensive effort into researching this. Just throwing this feature (I don't see how this could have happened accidentally) out, or disregarding it as undefined behavior, essentially reducing the scope of what Blazor can be, feels wrong to me.

ghost commented 1 year ago

Thanks for contacting us.

We're moving this issue to the .NET 8 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

SteveSandersonMS commented 1 year ago

We've been discussing this in some detail and come to the conclusions that:

So @LunicLynx, you do not have to worry that we are going to stop this from working. That is, at least, not our intention in the forseeable future. If we ever did change Blazor WebAssembly to have asynchronous event dispatch, it would be a big deal of a change and we would most likely have to give people the option of retaining the existing synchronous behavior for back-compat.

As for the questions about whether it is viable to create custom controls with Blazor - yes of course it is, that's very much the whole idea of a component-based UI framework. We can't break the laws of physics (and somehow make Blazor Server event handlers run synchronously with a client-side JS event) but we make things work as much as possible and try to make the developer experience as convenient as we can.

LunicLynx commented 1 year ago

Thanks for the feedback, that is completely understandable.

Regarding the laws of the physics quip. Here are the thoughts I had in mind, not saying this should be done mind you, just that it is possible.

Take this handler:

public async Task KeyDown(KeyboardEventArgs e)
{
   // do something with event args
   await SomeAsyncCall()
   // ....
}

The compiler splits this at the await into the synchronous part an the async continuation. It would be imaginable to split both parts into 2 assemblies. The first part send to the browser and executed synchronous in wasm, while the continuation is executed on the server.

With tree-shaking, the size of the assembly send to the browser would be very minimal. I would expect an attribute indicating, that this behavior should be happening, it only makes sense for certain kinds of handlers.

I know this has many implications, memory state transfer, references, etc. I'm going for possible here, not easy nor straight forward. And this wouldn't be pure server mode so probably no one would want that.

But I also see potential for a hybrid mode. Baking the transfer to server into the await call and clearly marking what can run in the browser.

Anyway, thanks again for the update.

Love all your hard work! .NET is awesome.

ghost commented 1 year ago

Thank you for contacting us. Due to a lack of activity on this discussion issue we're closing it in an effort to keep our backlog clean. If you believe there is a concern related to the ASP.NET Core framework, which hasn't been addressed yet, please file a new issue.

This issue will be locked after 30 more days of inactivity. If you still wish to discuss this subject after then, please create a new issue!