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

Simple Update for state management #29749

Closed
wants to merge 1 commit into from
Closed

Conversation

Runaho
Copy link

@Runaho Runaho commented Jul 10, 2023

In my own tests, using OnChange += StateHasChanged; unfortunately did not work.

Based on this, I did some research and came to the conclusion that when we write our own handler method, we can solve this problem by invoking asynchronously.

If we can update the document in this way, it will be better for people like me who come across this problem to learn directly with the already solved version.

Please write if you have any suggestions.
I would like to point out that I did not open an issue because I thought it was a simple suggestion.

In my own tests, using OnChange += StateHasChanged; unfortunately did not work.

Based on this, I did some research and came to the conclusion that when we write our own handler method, we can solve this problem with invoke async.

If we can update the document in this way, it will be better for people like me who come across this problem to learn directly with the already solved version.
@Runaho
Copy link
Author

Runaho commented Jul 10, 2023

@dotnet-policy-service agree

@guardrex guardrex self-assigned this Jul 10, 2023
@guardrex
Copy link
Collaborator

guardrex commented Jul 10, 2023

Hello @Runaho ... This isn't a patch update to non-working/poorly-implemented code. The original code works. Normally, we do prefer an issue to discuss significant changes. It sounds more like the code didn't work in your use case because you needed your own async handler, and perhaps what we should do is make this say that the demo code is a starting point for further customization (i.e., expand the NOTE on Lines 6-7).

@MackinnonBuck ... The section where this code appears is at ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/state-management?view=aspnetcore-7.0&pivots=server#in-memory-state-container-service-server

... and I just confirmed that it works as shown. Do you want to take a look, or do you think Steve or Javier should look at this?

Ultimately, we'll do one of the following ...

  • Accept the changes, perhaps with a few NIT updates.
    • Either only show it with @Runaho's changes.
    • Show both the original and @Runaho's approach SxS. The original works when no other async tasks are needed and only a StateHasChanged call is needed. @Runaho's approach is appropriate when a custom handler method is needed with async tasks to perform in the custom handler.
  • Just add that this is a starting point for further customization.
  • Close as a won't-fix.

@Runaho
Copy link
Author

Runaho commented Jul 10, 2023

I have prepared an update that you can try on this issue, I am leaving a simple example repo link.
There is also a video in Readme.
@guardrex

https://github.com/Runaho/BlazorStateManagement

@guardrex
Copy link
Collaborator

I think that's covered in the Rendering article in more detail ...

https://learn.microsoft.com/en-us/aspnet/core/blazor/components/rendering?view=aspnetcore-7.0#receiving-a-call-from-something-external-to-the-blazor-rendering-and-event-handling-system

I need to digest this scenario a bit more ... things like what happens if your SendMessage call in the component is wrapped in InvokeAsync to perhaps dispatch it. I'm not saying that will work. I'd like to compare and contrast the examples in the docs with your use case.

I'm working on a potentially complicated set of updates to our form docs, so I'll need to circle back around to this. I'll try to get back to it by tomorrow ...... but no promises on that! 😆🏃🏃🏃🏃😅

@Runaho
Copy link
Author

Runaho commented Jul 10, 2023

Thank you for your effort @guardrex
As far as I understand, when we look at the example, it has been added as scoped, not as a singleton, in Server Project so it will only work on the same client and will work correctly.
But when we want to trigger more than one client, this example causes an error.

Maybe we can add it to the document instead of editing it directly?

@guardrex
Copy link
Collaborator

Yes, that was the intent. IIRC, it wasn't meant to be a state container that works across circuits. We don't have that ... well ... we have the Blazor-SignalR approach that uses a SignalR hub and works like a SignalR app. These state management subjects that the docs discuss are more about state with one client, as you point out.

We'll have to see what Mackinnon thinks about this, perhaps in consultation with other engineering team members and Dan and Artak.

We're still cranking away on major updates for .NET 8, and that's going to keep us (🦖 ME! 😆) busy for the next few months. If we're going to set up a new, complete state management demo that works across circuits, then it might take a little more time to develop. I know that the product unit is too busy to help with anything significant at this time due to the .NET 8 work.

@guardrex
Copy link
Collaborator

guardrex commented Jul 10, 2023

I guess that I should also mention the loss of the Blazor Server app because I'm not sure if you're aware of it or not. "Blazor Server" as a thing is going away permanently as a templated app at 8.0 in favor of the new Blazor Web App. There will still be a circuit-based Blazor Server model running under it, but we wouldn't base new coverage on a Blazor Server app. It would be based on a Blazor Web App that can be server-side (SSR) or client-side (CSR) rendered. The state management thing that we would add would need to work in that context (i.e., an app that can automatically switch from SSR to CSR). State management for this scenario might not be much different in terms of a cross-circuit bits for SSR, but I think we'd need to figure out what to show for what happens when such an app goes into CSR. Anyway ... I just have the feeling that anything significant that we would do would be pushed into 2024 for this reason.

@Runaho
Copy link
Author

Runaho commented Jul 10, 2023

Got it, thank you.
The main goal here is to be able to trigger other clients with a singleton state container rather than creating a new hub since Blazor has already opened the SignalR socket.

When you look at it this way, it actually provides a really easy implementation.
This topic is no longer about simply editing documents :)

I think with this approach, we are already using the advantage of the Blazor Server application running on a single server and we are building a pusher-like structure.

A dynamic Blazor Rendering sounds really nice and it's a nice privilege to be able to use it as (SSR) or (CSR) when I want.

Maybe we can close this thread or turn it into something else after the other people you mentioned have expressed their opinion about it.

@guardrex
Copy link
Collaborator

We've scoped this issue to an ask for a more general state container service than what the topic provides, given that the article is chiefly concerned with state management for a single client (single circuit). To help unblock devs who hit the The current thread is not associated with the Dispatcher. error, this article should cross-link the Rendering article that explains how to deal with callbacks invoked outside of Blazor's synchronization context. I think it makes the most sense to compose the cross-link in a Troubleshoot section, as opposed to just having it in an Additional Resources section. I'm going to fix this issue that way. I'll also add a long-range tracking issue to review this issue for more work in 2024.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants