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

Implement .NET 8 Blazor state management #3641

Merged
merged 12 commits into from
Jan 3, 2024

Conversation

rockfordlhotka
Copy link
Member

@rockfordlhotka rockfordlhotka commented Dec 27, 2023

Closes #3596

@ossendorf-at-hoelscher
Copy link

Hi. Would you mind comments and questions from me to this pr changes?

@rockfordlhotka
Copy link
Member Author

rockfordlhotka commented Dec 28, 2023

I love feedback!

@ossendorf-at-hoelscher if you'd like to be able to use the PR feedback tools in GitHub, please send me a signed contributor agreement and I'll add you.

@ossendorf-at-hoelscher
Copy link

I love feedback!

@ossendorf-at-hoelscher if you'd like to be able to use the PR feedback tools in GitHub, please send me a signed contributor agreement and I'll add you.

I'll discuss this with my superior if I can get some worktime for such things. Otherwise I'll send one from my private account 😄

@rockfordlhotka
Copy link
Member Author

@ossendorf-at-hoelscher I made quite a few changes - some based on your feedback.

I had it working, and then did something that broke the transition from wasm back to server. And I don't know what I did to break it 😢

Some observations.

  1. I can't use ConcurrentDictionary because that appears to do a lock that blocks wasm->server transition
  2. Using a tcs also seemed to block the wasm->server transition
  3. Of course, 1 and 2 are now suspect, because it is currently broken anyway!
  4. I'd forgotten that the initial landing page must initialize the cookie, and so state is unavailable for the site's initial landing page. I've now added properties that can be used to determine whether the state is actually available (via StateManager)

I don't know how deeply you (or anyone else) wants to invest in helping to get this working, but just in case, I have pushed my test project to GitHub:

https://github.com/rockfordlhotka/CslaBlazorApp

The solution directly references the Csla, Csla.AspNetCore, Csla.Blazor, and Csla.Blazor.WebAssembly projects. To help load the solution, here's my relative directory structure:

s:\src\CslaBlazorApp
s:\src\rdl\csla

If you clone those two repos into that folder structure, the solution should just load and run.

Unfortunately, right now, navigating from the Counter page (once it reloads as wasm) back to either server-rendered page will result in a timeout because the Counter page's Dispose method isn't invoking asynchronously like it should. It is being blocked by the initialize process of the server pages.

I can't figure out why right now. My Blazor8State app is implemented basically the same exact way, and it works fine (as did the CslaBlazorApp at one point in here).

@StefanOssendorf
Copy link
Contributor

@rockfordlhotka A short analysis shows that your session.IsCheckedOut does not transition from true (used in WASM) to false (used on Server) when you have such a switch between pages. Is this bool really necessary?

@StefanOssendorf
Copy link
Contributor

I suppose I should have read your comment more carefully 😅.

The Counters Dispose method is invoked when navigating from Counter -> Weather. But the updated session (on server) is never(?) updated on the client side. So you run into the timeout. Furthermore it's not a deadlock. It's just that no state change comes back after updating it on the server.

On that regard: You should change the Blazor StateManager from void SaveState to Task SaveState and await the Task of SendSession(). Otherwise you get timing problems which will be hard to track down.

@rockfordlhotka
Copy link
Member Author

The goal is to support LocalContext, and the developer can change those values as they choose. So any changes made in wasm should be available on the server (and visa versa).

With my Blazor8State app I was able to make this work using the ischeckedout flag and the Dispose in the wasm page.

I momentarily had it working in CSLA as well, and really wish I'd done a git commit at that point!

@rockfordlhotka
Copy link
Member Author

Furthermore it's not a deadlock

Sadly it really is. The wasm page isn't disposed because the server page is blocking the transition.

If the server page doesn't block, then dispose is called, the state is updated on the server, and the while loop exits.

@StefanOssendorf
Copy link
Contributor

StefanOssendorf commented Jan 2, 2024 via email

@rockfordlhotka
Copy link
Member Author

Now you see why I added the timeout, otherwise the browser is permanently locked.

Run the Blazor8State app to see how it works, and why I'm puzzled that the csla app fails.

@rockfordlhotka
Copy link
Member Author

I figured it out! Finally!

I was meticulously comparing my working Blazor8State code to the not working CslaBlazorApp code. Line by line, undoing all sorts of nice improvements, etc.

Finally realized that the server-static page in the working code is streaming. That's the key, is to have the server-static page be streaming, so it starts rendering and thus releases the wasm page to dispose.

@StefanOssendorf
Copy link
Contributor

Great. So back to implementing the nice improvments :D

@StefanOssendorf
Copy link
Contributor

Since the ConcurrentDictionary/ TaskCompletionSource wasn't the problem wouldn't it be better to try them now (with a working solution)?

@rockfordlhotka rockfordlhotka merged commit 1851319 into MarimerLLC:main Jan 3, 2024
1 check passed
@rockfordlhotka rockfordlhotka deleted the 3395-tests branch January 3, 2024 01:43
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.

Support LocalContext and ClientContext with the Blazor 8 auto-render model
3 participants