Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design proposal: solution for cascading values into interactive pages #49104

Closed
SteveSandersonMS opened this issue Jun 30, 2023 · 3 comments
Closed
Assignees
Labels
area-blazor Includes: Blazor, Razor Components design-proposal This issue represents a design proposal for a different issue, linked in the description
Milestone

Comments

@SteveSandersonMS
Copy link
Member

SteveSandersonMS commented Jun 30, 2023

There's a gap in the Blazor SSR+interactive design currently, which is that whenever you have a @page component with Server or WebAssembly rendermode, that page component runs outside the context where it would receive any cascaded values, which means:

  1. Authentication state is always empty (In blazor SSR, when setting @attribute [RenderModeServer], cascading parameter Task<AuthenticationState> always is null #48927)
  2. You can't receive [SupplyParameterFromQuery] (and presumably also [SupplyParameterFromForm])
  3. You can't receive any custom cascading values that you've added for app-specific purposes

Obviously this is something we have to address, especially because of item 2 there which is the most basic scenario among them.

Proposal

We can provide a way of registering root-level cascading values independently of the component hierarchy. This makes sense whenever there are values that are effectively global within a given circuit/wasm scope. Then:

  • Those will be available everywhere, whether or not a @page component is being rendered as an island or if it's embedded inside an App component or a RouteView or whatever else.
  • They will still be real cascading values as far as the recipient is concerned, so no change in programming model there. The same update notifications can still occur.
  • They can still be overridden at child levels in the hierarchy.
  • We can simplify App components, RouteView, etc., by not having to wrap multiple layers of CascadingValue around things.

As for implementation, I've done a quick experiment and found this works very naturally if we make the DI system do most of the work:

  • Have a way of registering any number of scoped cascading value supplier services
  • When CascadingParameterState recurses up to the root of the component hierarchy and didn't find a value yet, iterate through the cascading value suppliers and let each of those be the supplier if they want

For the API, I propose we restrict this to exposing only the exact same capabilities that are currently exposed through CascadingValue. That is, ICascadingValueSupplier would continue to be internal, but there would be APIs like:

// Registers an ICascadingValueSupplier that has IsFixed=true and always supplies this specific value, using the same CanSupplyValue rules as CascadingValue
services.AddCascadingValue(sp => new MyCascadedThing { Value = 123 });

// Registers an ICascadingValueSupplier that has IsFixed=false. Initially it supplies the specified value, but you can later
// call NotifyChanged if you want to trigger a value update.
services.AddCascadingValue(sp =>
{
    var thing = new MyCascadedThing { Value = 456 };
    var source = new CascadingValueSource<MyCascadedThing>(thing, isFixed: false);

    // Artificial example of triggering an update. You can mutate the existing value in place and call NotifyChanged(),
    /// or you can supply a whole new value instance (again, these options are exactly like the CascadingValue component)
    Task.Run(async () =>
    {
        await Task.Delay(3000);
        source.NotifyChanged(new MyCascadedThing { Value = 1000 });
    });

    return source;
});

As you see, the design is oriented around providing the exact same capabilities, not more or less, than a <CascadingValue> component at the root. So it will always be possible to replace root-level <CascadingValue> with one of these registrations if you want to make it global.

Then it will be pretty trivial to create a variant of <CascadingAuthenticationState> as a DI extension method like services.AddCascadingAuthenticationState() which sets up an equivalent registration, and uses NotifyChanged to trigger updates like <CascadingAuthenticationState> would do.

Likewise, the default set of Blazor DI services can include cascading value suppliers for Form and Query values, so they will work everywhere, and we can decouple this from RouteView.

This will be a relatively cheap solution to the overall problem. I have a working version of it already (for case 3 - custom values - I haven't validated the end-to-end with SupplyParameterFromQuery etc but it seems like it should work). The only unexpected complication was that NotifyChanged has to use Dispatcher.InvokeAsync internally, but with that in place things work correctly.

Limitations/drawbacks

Ordering

I'm not proposing we define any way of ordering the DI-registered cascading value suppliers. Developers will be responsible for registering types that are specific enough that they don't interfere with unrelated cascading value suppliers, or to use the optional name parameter to be even more explicit. That won't be a problem for the built-in ones as they can use the full capabilities of CanSupplyValue to guarantee they only supply values for the correct type of recipient.

This is mostly equivalent to what happens with cascading values in the component hierarchy, albeit with less explicit control. If this later turns out to be problematic, we can optionally add an Order property to the registrations - it's not a perfect solution as then you have to coordinate order values, but it would offer more control. I don't think we have to do that initially.

Disposal

Just like CascadingValue doesn't dispose any values you give to it, I don't propose that we would auto-dispose any values you supply through these APIs. Developers already have multiple ways of doing that.

Scope/lifetime control

One of the nice things here is that because ICascadingValueSupplier is internal, the only possible way to register one in DI is to use the method being proposed here, and so the framework can control the DI lifetime. I propose that we only ever register as "scoped", because other options are problematic (if we allow singleton, that creates a new kind of scenario where unrelated user circuits can be subscribed to the same cascading value, and if we allow transient, it would be completely unlike other cascading values).

pinging @dotnet/aspnet-blazor-eng for feedback or alternatives

@SteveSandersonMS SteveSandersonMS added the area-blazor Includes: Blazor, Razor Components label Jun 30, 2023
@javiercn
Copy link
Member

Ordering
I'm not proposing we define any way of ordering the DI-registered cascading value suppliers. Developers will be responsible for registering types that are specific enough that they don't interfere with unrelated cascading value suppliers, or to use the optional name parameter to be even more explicit. That won't be a problem for the built-in ones as they can use the full capabilities of CanSupplyValue to guarantee they only supply values for the correct type of recipient.

This is mostly equivalent to what happens with cascading values in the component hierarchy, albeit with less explicit control. If this later turns out to be problematic, we can optionally add an Order property to the registrations - it's not a perfect solution as then you have to coordinate order values, but it would offer more control. I don't think we have to do that initially.

If we think we might need ordering, we should include it from the beginning. Introducing it at a later stage will be breaking to apps.

We should in general avoid relying on DI for ordering, and it can cause issues that are hard to debug. For example, moving the order of your services.AddXXX can have unintended side-effects.

Is adding cascading values to the root, something that is limited to the renderer? Or is that something that users could be able to do.

@SteveSandersonMS
Copy link
Member Author

We should in general avoid relying on DI for ordering, and it can cause issues that are hard to debug. For example, moving the order of your services.AddXXX can have unintended side-effects.

I agree, hence the notion that correct usage of this feature entails registering things that are specific enough they won't clash.

I don't think adding Order later would be breaking. At that stage we could treat all unset order values as zero, producing the same behavior as we have today (assuming we do a stable sort, if we sort). Then people choosing to add an Order somewhere are explicitly choosing to make that one registration either higher or lower prioritized than it was before.

Is adding cascading values to the root, something that is limited to the renderer? Or is that something that users could be able to do.

To cover scenario 3 in the top description, it has to be something that can be done publicly. We've always encouraged people to use CascadingValue for top-level values that can trigger change notifications (like authentication state does, but also for other things like "current theme"), so if that mechanism is to have an equivalent with SSR and @page, there would need to be an equivalent. This will be necessary for things like component libraries that establish global context through cascaded values.

@mkArtakMSFT mkArtakMSFT added this to the 8.0-preview7 milestone Jul 3, 2023
@mkArtakMSFT mkArtakMSFT added the design-proposal This issue represents a design proposal for a different issue, linked in the description label Jul 3, 2023
@SteveSandersonMS
Copy link
Member Author

Done in #49204

@ghost ghost locked as resolved and limited conversation to collaborators Aug 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components design-proposal This issue represents a design proposal for a different issue, linked in the description
Projects
None yet
Development

No branches or pull requests

3 participants