Cascading values+params for state management

[guardrex]

Fixes #34236

Thanks @hakenr! 🎷 ... This is probably a bit rough in spots. If you have time to look, I'll address your feedback before pinging a PU engineer. If you're busy, no worries ... I'll get one of them on here early next week to put an 👁️ on this.

UPDATE (Monday, 2/24): I pinged to ask if either Illona or Ondrej are available to take a look. If they're busy, I'll hit up either Pavel or Marek to look.

Notes

• Add a CascadingValueSource<T> with notifications (INotifyPropertyChanged/NotifyChangedAsync) approach to the Cascading values and parameters article. The basis for the example is code provided by @kzu on CascadingValueSource<T> should automatically invoke NotifyChangedAsync for INotifyPropertyChanged values aspnetcore#53257 (Thanks @kzu! 🎸) . Sanderson said it's supported at CascadingValueSource<T> should automatically invoke NotifyChangedAsync for INotifyPropertyChanged values aspnetcore#53257 (comment), but he also commented elsewhere that "In practice it's extremely rare for people to want to do INotifyPropertyChanged with Blazor." However, that might be because it isn't documented (until now here 😄).

• I favor placing this coverage because it's easy to understand/implement and flexible/powerful with a class type. I demo both setting a property value directly and having a method in the class that updates a property. An app can process state quite easily using this approach. I make sure to call out that there's a perf hit and to be careful with this approach.

• Add a short section to the State Management article mentioning cascading values and parameters generally. For >=8.0, cross-link the root-level cascading value/CascadingValueSource<T> /INotifyPropertyChanged/NotifyChangedAsync approach.

• WRT ...

  the entire State Management page could simply be a list of approaches with links to other pages that explain each feature in detail

  My initial concern is simply that explaining the features and explaining state management using the features in one spot is problematic because state management is a more advanced use case for the two or three scenarios here where this idea applies. For root-level cascading values with notifications, this approach seems to work well because the use case is a state management approach. I've made a tracking item entry for this idea to look at it again later.

---

Internal previews

📄 File	🔗 Preview link
aspnetcore/blazor/components/cascading-values-and-parameters.md	aspnetcore/blazor/components/cascading-values-and-parameters
aspnetcore/blazor/state-management.md	aspnetcore/blazor/state-management

[guardrex]

Illona sent a better factory that unhooks the event handler. Unfortunately, what she sent isn't running correctly yet. I'll bang the keyboard for a bit 🐒 and see if I can get it running.

I tried subclassing CascadingValueSource<T> without luck (yet 😄). I'll try again tomorrow with it.

[guardrex]

🎉 Better! ... This works .............. playing off of Illona's remark that the handler should be disconnected and your, @hakenr, remark that perhaps basing it on the Auth State Provider framework code approach would be good ......

using System.ComponentModel;
using Microsoft.AspNetCore.Components;

namespace Microsoft.Extensions.DependencyInjection;

public static class CascadingApplicationStateServiceCollectionExtensions
{
    public static IServiceCollection AddCascadingApplicationState<T>(
        this IServiceCollection serviceCollection, T value, bool isFixed = false)
        where T : INotifyPropertyChanged
    {
        return serviceCollection.AddCascadingValue<T>(services =>
        {
            return new ApplicationStateCascadingValueSource<T>(value, isFixed);
        });
    }

    private sealed class ApplicationStateCascadingValueSource<T>
        : CascadingValueSource<T>, IDisposable where T : INotifyPropertyChanged
    {
        private readonly T value;
        private readonly CascadingValueSource<T> source;

        public ApplicationStateCascadingValueSource(T value, bool isFixed = false)
            : base(value, isFixed = false)
        {
            this.value = value;
            source = new CascadingValueSource<T>(value, isFixed);
            this.value.PropertyChanged += HandlePropertyChanged;
        }

        private void HandlePropertyChanged(object? sender, PropertyChangedEventArgs e)
        {
            _ = NotifyChangedAsync();
        }

        public void Dispose()
        {
            value.PropertyChanged -= HandlePropertyChanged;
        }
    }
}

... with ...

builder.Services.AddCascadingApplicationState<NotifyingDalek>(new NotifyingDalek() { Units = 888 });

[guardrex]

I put it on the PR and will revise further on Tuesday morning.

[hakenr]

@guardrex I like the samples. The only thing that worries me is the name of the AddCascadingApplicationState<T>() method, which suggests the possibility of using it as a large state storage (aka store) with many rich properties representing different parts of the global application state.

What we need to emphasize is that ANY change in such state (ANY property) will cause ALL subscribed components to re-render, regardless of which part of the state they use. So, we should guide users to avoid creating a single large class representing the entire global application state. Instead, they should create more granular classes and cascade them separately (to allow more specific subscriptions with cascading parameters), ensuring that only components subscribed to a specific portion of the application state are affected by changes.

[guardrex]

Totally agree! I'll probably even STEAL your language 👮‍♂️🚓😄 when I make changes tomorrow to react to that in the code examples. The thing that I was just trying to accomplish today was creating a better approach with unhooking the handler and making it a service collection extension. So far, so good!

[ilonatommy]

Looking good!

[guardrex]

Illona suggested naming ...

• AddCascadingStateNotifier 👈 I'll go with this on the next commit.
• AddCascadingStateSubscription
• AddCascadingScopedState
• AddCascadingState

I'll be working further on this PR shortly.

[guardrex]

Made the updates.

@hakenr ... I placed your remarks in two spots for the greatest visibility ...

• Where NotifyingDalek is described. The remarks directly pertain to the class.
• At the very end of the section. It's an important concept that we'd like to leave them with at the end to reinforce the point.

The code updates are verifying working here; so unless there are additional change requests (e.g., naming in the code), I'm ready to merge this.

[guardrex]

@hakenr ... I think that gets everything, except ...

The registration should instead look something like this, leveraging generic type inference:

builder.Services.AddNotifyingCascadingValue(new NotifyingDalek() { Units = 888 });

...we are actually registering a cascading value, which should be clear.

I think you must mean that the signature of the method (and method body) has to change because it can't merely be calling it that way ... if I remove the type, it no-ops at runtime.

If you have a different signature/method in mind that will resolve it, then it will need to be supplied to me. I'm just an LOB 🦖 who knows a lot about education and technical writing but "faking it until he makes it" 🙈😆 with some of the coding aspects. I leave the heavy code lifting to the engineering gurus, such as yourself and the framework engineers (@ilonatommy, for example). If you'd like @ilonatommy to assist with resolving that aspect, then I defer to her, or she might be too busy and need to ping one of the other engineers.

[hakenr]

I think the method definition should remain as is:

public static IServiceCollection AddNotifyingCascadingValue<T>(
    this IServiceCollection services, T state, bool isFixed = false)
    where T : INotifyPropertyChanged
{
    return services.AddCascadingValue(sp =>
    {
        return new CascadingStateValueSource<T>(state, isFixed);
    });
}

I just believe (and I haven't tested it 😇) that these two lines are equivalent, with the generic type inferred by the C# compiler:

builder.Services.AddNotifyingCascadingValue(new NotifyingDalek() { Units = 888 });
builder.Services.AddNotifyingCascadingValue<NotifyingDalek>(new NotifyingDalek() { Units = 888 });

You should be able to verify this by hovering over the shortened call in Visual Studio - it should show the full version in the tooltip.

There must be some other issue if there's a "no-op at runtime."
Do you want me to double-check the whole sample, or would you like to give it another try on your end?

(BTW, I don't insist on the shortened call - it's just more readable. What worries me is: How the hell can the shortened call cause a no-op? There must be something else going on...)

[guardrex]

There must be some other issue if there's a "no-op at runtime."

Yes ... I think that there's something else going on in the framework. I checked it a few times, and it indeed no-ops without the type. See how the initial value isn't appearing ...

I see what you mean about hovering over it ...

It's nice to hear that my lack of knowledge was for something that doesn't exist in the first place. 😆 Indeed, the sig on the PR was the only way that I knew the method signature should be composed.

I guess we won't hold this up over the shortened syntax. I'm ready to go ahead and merge it.

[guardrex]

I resolved the problem! 🎉 It was missing a T type ...

- return services.AddCascadingValue(sp =>
+ return services.AddCascadingValue<T>(sp =>

After that, it's working again and with the shortened syntax ...

builder.Services.AddNotifyingCascadingValue(new NotifyingDalek() { Units = 888 });

I'll merge this now!

Thanks @hakenr and @ilonatommy! 🍻