Skip to content

Conversation

@MackinnonBuck
Copy link
Member

@MackinnonBuck MackinnonBuck commented Aug 9, 2021

Dynamic root components EventCallback parameter support
JavaScript functions can now be passed as parameters to Blazor dynamic root components. The motivating scenario for this is allowing event callbacks in Blazor components to be handled by functions defined in JavaScript. The supported .NET parameter types are EventCallback and EventCallback<TValue>.

PR Description
Using this feature is as simple as passing a normal JS function as a parameter to a dynamic root component. Under the hood, parameters of 'function' type are wrapped in JSObjectReference instances before being sent to .NET. The lifetime of these instances is automatically scoped to the lifetime of the dynamic root component.

On the .NET side, deserializing parameters into types EventCallback and EventCallback<TValue> is special-cased; the parameters are assigned EventCallbacks that invoke the provided JS function from the deserialized JSObjectReference.

Fulfills some requirements for #35112

@MackinnonBuck MackinnonBuck requested a review from a team as a code owner August 9, 2021 23:58
@ghost ghost added the area-blazor Includes: Blazor, Razor Components label Aug 9, 2021
@MackinnonBuck MackinnonBuck changed the title Dynamic root components 'Func' parameter support Dynamic root components 'EventCallback' parameter support Aug 10, 2021
@MackinnonBuck
Copy link
Member Author

@SteveSandersonMS I've made some changes to centralize the code that determines whether a parameter is an event callback. My approach may have differed slightly from what you had in mind, so let me know if you would prefer it be done in a different way.

The biggest change I made to get this to work was providing component parameter info to JS in enableJSRootComponents regardless of whether an initializer was provided. This allows DynamicRootComponent to determine if a parameter is of type eventcallback from the decision made earlier in .NET. This required some minor refactoring on the JS side and slightly more on the .NET side.

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Aug 11, 2021

This is looking really good to me!

My approach may have differed slightly from what you had in mind, so let me know if you would prefer it be done in a different way.

No, looks solid.

The biggest change I made to get this to work was providing component parameter info to JS in enableJSRootComponents regardless of whether an initializer was provided.

I admit I hadn't thought through that particular consequence, but it doesn't seem problematic and the way you've implemented it is clear. Thinking about it more, the previous "optimization" about not supplying parameter infos when there was no initializer would only take effect in really edge-case scenarios, and it would probably be inevitable in the long run that we'd have reasons to pass through parameter infos for whatever future functionality we end up building in this area. So in retrospect I don't think there was much of a good reason to be trying to skip passing that info, and what you have done here is clearer than we had before.

@MackinnonBuck
Copy link
Member Author

@SteveSandersonMS Thanks for the feedback! I've just updated the implementation to support the parameter reassignment scenarios (including setting parameters to null). The new approach also doesn't mutate the parameters argument and instead creates a copy.

Copy link
Member

@SteveSandersonMS SteveSandersonMS left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic - this looks ideal! Thanks for getting it into such good shape.

@MackinnonBuck MackinnonBuck enabled auto-merge (squash) August 12, 2021 20:34
@MackinnonBuck MackinnonBuck merged commit 24ea7ef into main Aug 12, 2021
@MackinnonBuck MackinnonBuck deleted the t-mbuck/js-components-eventcallback branch August 12, 2021 22:22
@ghost ghost added this to the 6.0-rc1 milestone Aug 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-blazor Includes: Blazor, Razor Components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants