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

Blazor: In-memory state container as cascading parameter #27296

Closed
lonix1 opened this issue Oct 15, 2022 · 16 comments · Fixed by #27455 or #27456
Closed

Blazor: In-memory state container as cascading parameter #27296

lonix1 opened this issue Oct 15, 2022 · 16 comments · Fixed by #27455 or #27456
Assignees
Labels
Blazor doc-enhancement Pri3 Source - Docs.ms Docs Customer feedback via GitHub Issue

Comments

@lonix1
Copy link

lonix1 commented Oct 15, 2022

[EDIT by guardrex: The current approach for the in-memory state container service is located at In-memory state container service.]

The "In-memory state container service" section shows a state container as a service.

For simple app-wide stuff, it is simpler to use a cascading value. I posted a related question (with working code) to StackOverflow. (UPDATE: "MrC" posted another very nice solution.)

For example, working code:

AppState.razor

<CascadingValue Value="this">
  @ChildContent
</CascadingValue>

@code {

  private bool _isDirty;

  [Parameter] public RenderFragment ChildContent { get; set; }

  protected override async Task OnAfterRenderAsync(bool firstRender) {
    await base.OnAfterRenderAsync(firstRender);
    if (firstRender) {
      await LoadStateFromLocalStorage();
    }
    if (!firstRender && _isDirty) {
      await SaveStateToLocalStorage();
      _isDirty = false;
    }
  }

  private bool _isDarkMode;
  public bool IsDarkMode {
    get {
      return _isDarkMode;
    }
    set {
      if (value == _isDarkMode) return;
      _isDarkMode = value;
      StateHasChanged();
      _isDirty = true;
    }
  }

  //other properties...

  private async Task LoadStateFromLocalStorage() {
    Console.WriteLine("LOADED!");
    await Task.CompletedTask;
  }

  private async Task SaveStateToLocalStorage() {
    Console.WriteLine("SAVED!");
    await Task.CompletedTask;
  }

}

App.razor

<AppState>
  <Router>
    ...
  </Router>
</AppState>                                                          

MyComponent.razor

<!--- markup --->

@code {
  [CascadingParameter] public AppState AppState { get; set; }

  //...
}

It would be nice to have that as another section, e.g. "In-memory state container as cascading parameter".


Document Details

Do not edit this section. It is required for learn.microsoft.com ➟ GitHub issue linking.

@guardrex
Copy link
Collaborator

Thanks for sending this in, @lonix1. We will take a look at it. However, we're booked with a lot of high priority work right through the end of the year, so we won't be able to reach this until early 2023. It won't get lost tho! It will be on a tracking issue.

@lonix1
Copy link
Author

lonix1 commented Oct 16, 2022

No worries - I'm using the above to great effect. It may help others if offered as an alternative.

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

Here's the @ShaunCurtis approach posted at SO.

Reference: https://stackoverflow.com/a/74080605

Here's a different solution that I think resolves most of the issues in your implementation. It loosely based on the way EditContext works.

First separate out the data from the component. Note the Action delegate that is raised whenever a parameter change takes place. This is basically the StateContainer in the linked MSDocs article.

public class SPAStateContext
{
    private bool _darkMode;
    public bool DarkMode
    {
        get => _darkMode;
        set
        {
            if (value != _darkMode)
            {
                _darkMode = value;
                this.NotifyStateChanged();
            }
        }
    }

    public Action? StateChanged;

    private void NotifyStateChanged()
        => this.StateChanged?.Invoke();
}

Now the State Manager Component.

  1. We cascade the SPAStateContext not the component itself which is far safer (and cheaper).
  2. We register a fire and forget handler on StateChanged. This can be async as the invocation is fire and forget.
@implements IDisposable

<CascadingValue Value=this.data>
    @ChildContent
</CascadingValue>

@code {
    private readonly SPAStateContext data = new SPAStateContext();

    [Parameter] public RenderFragment? ChildContent { get; set; }

    protected override void OnInitialized()
        => data.StateChanged += OnStateChanged;

    private Action? StateChanged;

    // This implements the async void pattern 
    // it should only be used in specific circumstances such as here in a fire and forget event handler
    private async void OnStateChanged()
    {
        // Do your async work
        // In your case do your state management saving
        await SaveStateToLocalStorage();
    }

    protected override async Task OnAfterRenderAsync(bool firstRender)
    {
        if (firstRender)
            await LoadStateFromLocalStorage();
    }

    public void Dispose()
        => data.StateChanged -= OnStateChanged;
}

Additional context is in the discussion on the SO comment Q&A.

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

@lonix1 ... AFAICT, we already have this approach covered (generally anyway ... i.e., a state-providing cascaded component) at ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/state-management?view=aspnetcore-6.0&pivots=server#factor-out-the-state-preservation-to-a-common-location

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

Yeah 🤔 ... That's close enough conceptually that we don't need to take action on this. This issue will remain associated with the topic on GitHub, so if devs select the View all page feedback link, they'll see this issue and can see the other example approaches/discussion from there.

Thanks for writing in on it. 👍

@guardrex guardrex closed this as completed Nov 1, 2022
@lonix1
Copy link
Author

lonix1 commented Nov 1, 2022

"View all page feedback" link - amazingly useful feature. It's buried at the very bottom so I never noticed it until now. Thanks.

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

Yeah ... I don't like what they did with that because it would have helped us probably HUNDREDS of times both with readers looking for things here and even US ... the doc 🐈🐈🐈 ... I've forgotten about it myself tucked way down there. Anyway ... that's where they want it, so that's where it will stay.

@lonix1
Copy link
Author

lonix1 commented Nov 1, 2022

I didn't notice the "Factor out the state preservation to a common location" section that you pointed me to, because it's only shown when choosing the blazor server option (I'm using wasm).

If it covers the same idea as what I posted above, and is applicable to wasm as well, maybe it should appear for wasm too?

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

Hummmmmmmm 🤔 ... Perhaps SO! 😄

I'll take a look.

@guardrex guardrex reopened this Nov 1, 2022
@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

The specific use case for that section uses ...

Microsoft.AspNetCore.Components.Server.ProtectedBrowserStorage

... for Blazor Server apps (which is now baked into the framework).

In this case, it basically just falls back to the main topic on the approach for WASM at ...

ASP.NET Core Blazor cascading values and parameters
https://learn.microsoft.com/aspnet/core/blazor/components/cascading-values-and-parameters

For the WASM scenario, the PU ... Steve IIRC ... only says the following in the topic ...

localStorage and sessionStorage can be used in Blazor WebAssembly apps but only by writing custom code or using a third-party package.

... because he didn't want to show an opinionated, general approach.

I'll add a cross-link there to the cascading values/params topic, but we're still going to leave it up to the dev to implement the code on their own.

@ShaunCurtis
Copy link

ShaunCurtis commented Nov 1, 2022

I had never seen that section either. Having studied it I have the same issue as with the initial code in the SO question. It's cascading this - CounterStateProvider - a great big fat class with lots of functionality that should not be exposed in sub-components. The point of my code was to cascade a lean object with just the functionality required?

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

I can't second-guess Steve on that tho. What's there came directly from the product unit, so I must assume that that's what he wants devs to do in general cases. Of course, devs can take another ... leaner ... approach if that suits them.

They felt good enough about the way ProtectedBrowserStorage was working to even place it into the shared framework for 5.0 without calling for any significant changes to the docs on using it. IIRC, we just went from 'use the package' to 'use it from the shared framework' type of changes a la ...

aspnet/Announcements#436

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

Actually, it was Pranav who directly signed off on the changes, which were kind'a minimal really ...

#20351

We can't ask Pranav btw ... he's not on this PU team any longer.

Looking back further, it was Mackinnion who provided the major review feedback on ...

#19287

... from the content that IIRC Steve provided earlier.

Still tho, I don't think I want to ping on this. If the PU is getting a number of PU requests for help on the matter, they'll let me know. For now, I'd like to stick with the View all page feedback for the discussion and approach concepts.

We're crazy busy 🏃😅 with the .NET 7 release, then I have a huge block of work to do on the Blazor security node for all of EOY right thru 🎅 Day and around 🎆 Day, then ... major topic overhauls and updates for 23Q1 ⛰️⛏️😅. I'd like to have a lean backlog going forward. I'm 👂 from the PU if they want to provide more coverage on this particular aspect.

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

I'll tell you what ... I'll compromise right now. Let's cross-link THIS ISSUE right in the topic section. That way, it will surface better for devs. At some point in the future ... next year! 😆 lol ... I'll ask Dan and Artak about it.

@lonix1
Copy link
Author

lonix1 commented Nov 1, 2022

The compromise is sensible.

However note when reconsidering this next year/whenever:
The text as it's written now discusses localstorage, and that's where the issue above stems from. But @ShaunCurtis' and my solutions are not dependent on localstorage. They are just ways to create a state container based on cascading parameters. Persistence to localstorage is optional, and up to the reader.

@guardrex
Copy link
Collaborator

guardrex commented Nov 1, 2022

The topic went beyond it's original purpose when the "in-memory state provider" was added. In the beginning, it was just focused on when user state held in the browser's memory is lost. Now, it's unfortunately a bit scatter 🧠. I've added this issue to a tracking issue so that I make sure to discuss this with the PU early next year.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blazor doc-enhancement Pri3 Source - Docs.ms Docs Customer feedback via GitHub Issue
Projects
Archived in project
4 participants