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

Possible fix for .NET 8 Blazor Web App breakage #481

Closed
wants to merge 7 commits into from

Conversation

WhitWaldo
Copy link

Adds .NET 8 as a valid target in addition to .NET 7 and the other versions.

Also moves state initialization out from OnAfterRenderAsync (which doesn't run on server anymore per the docs) to OnInitializedAsync which runs after existing logic in OnInitialized and after OnParametersSet (which in .NET 8 now runs before OnInitialized{Async}), so it's the last lifecycle method before OnAfterRender, assuming it were still being called.

I'm having problems getting Fluxor.Blazor.Web to build on my machine (likely missing one of your targets) so I haven't yet tested this, but as covered in the related issue,, when I set a breakpoint in the ActionDispatched method, I can see that the actions are accumulating in the Store and they don't dequeue because the store never sets HasActivatedStore to true, which only happens via this call to Store.InitializeAsync() which only happens in OnAfterRenderAsync which isn't called on the server anymore, so it intuitively feels like this might fix the problem.

I'll follow up if I'm able to validate it locally furhter.

@WhitWaldo
Copy link
Author

WhitWaldo commented Mar 8, 2024

There's unfortunately more that needs tackling here. I built a local version of Fluxor that only target .NET 8 to simplify this and it builds fine now, but while the Counter page loads without error when @rendermode InteractiveServer and I see the DequeueActions method trigger as expected, when I swap this out with @rendermode InteractiveAuto, now I'm getting a DI-related exception:

Error: One or more errors occurred. (Cannot provide a value for property 'State' on type 'Blazor8Test.Client.Pages.Counter'. There is no registered service of type 'Fluxor.IState`1[Shared.State.CounterUseCase.CounterState]'.)

This is odd since I'm registering all dependencies on the primary project so it should flow through to the client (per the guidance here) but this remains a work in progress.

@mrpmorris
Copy link
Owner

Fancy pairing on this one if the days?

I'm available from 5PM week days (UK time)

@WhitWaldo
Copy link
Author

WhitWaldo commented Mar 8, 2024

Sure - we can sync on that next week. I'm in the US, but if I did the math right, that puts me at 11 AM or later, so that's more than fine.

I've identified at least part of the problem.

In .NET 7, you have either a server or a WASM project. You register Fluxor via the dependency injection scheme and initialize the store by placing the <StoreInitializer> component at the top of what was then the App.razor file and the rest works like magic.

In .NET 8, you cannot quite do this because as the library author, you're not going to know what render mode the user is opting for as it can be done on a per-page/component basis instead of globally, so you've got to be a bit more flexible about how the service is initialized.

On the Server project, the <StoreInitializer> should be placed at the top of the Routes.razor file as App.razor is now what was the _Host.cshtml file. Both the server and the client need to perform the Fluxor DI registration, so this should be either put in a shared project or placed in the client app and called by both. Coupled these two with the change I made to initialize the store in OnInitializedAsync instead of in OnAfterRender to avoid conflicts with any pre-rendering.

However, on the client, we've got several limitations to consider:

  • There's no such thing as scoped DI registrations - all scoped registrations are just singleton registrations.
  • There's no "globally run" file like that of Routes.razor to run initialization off of.

As such, on the Client project, in addition to the local Fluxor DI registration mentioned above, I propose that because SetParametersAsync now runs before OnInitializedAsync in the component lifecycle, the FluxorComponent be updated to override OnParametersSetAsync with the following:

        /// <summary>
        /// Method invoked when the component has received parameters from its parent in
        /// the render tree, and the incoming values have been assigned to properties.
        /// </summary>
        /// <returns>A <see cref="T:System.Threading.Tasks.Task" /> representing any asynchronous operation.</returns>
        protected override async Task OnParametersSetAsync()
      {
	    await base.OnParametersSetAsync();

            //Attempt to initialize the store knowing that if it's already been initialized, this won't do anything.
            await Store.InitializeAsync();
        }

As the Store has a flag checking if it's already been initialized, if this runs and that flag is true, nothing will happen, but because all Fluxor components are expected to inherit a FluxorComponent this will ensure that the store is initialized and starts to dequeue pending actions as expected.

There's no need to change the FluxorOptions to prefer a Singleton over a Scoped registration (as the Client is going to treat is as a Singleton anyway). There's also no need to disable prerender to make this work. I've tested that it works on interactive auto whether prerender is enabled or not.

I've tested this several times and it now works precisely as expected on both the Server and, after waiting for it to load the WASM bundle, switching pages and switching back to validate it's disconnected from the server, successfully verified it works on the Client as well. I'll update the PR shortly with my proposed changes.

Remaining open work:

  • Might be worth rethinking what needs to happen in StoreInitializer as it's only running on the server and not the client project. I might suggest tentatively shoveling that into a singleton service with a HasInitializedFlag that's called from any component implementing FluxorComponent and then recommending that best practices be that all Fluxor-capable pages/components inherit from FluxorComponent instead of making it semi-optional as today so that the experience is identical on both server and client projects.
  • In the same vein, JS middlewares only register on the server since they're not initialized on the client.
  • Because the registration in server and client differ and there's no other way I've identified to share the state between the different contexts, I think this is going to require some sort of persistence/rehydration layer not unlike what redux-persist and redux-store achieve. I'm working on a proof of concept that at least demonstrates this idea working.

@WhitWaldo
Copy link
Author

I've added a single persistence layer to Fluxor that appears to resolve the cross-rendermode issue. As observed by others on another thread about this, it's one thing to get Fluxor up and running again, but a whole other thing to completely lose your state as soon as you jump render modes (e.g. server -> web assembly).

While I've taken a stab at implementing this in a lightweight manner, it wasn't until I was integrating the changes from my test project to this branch that I realized my solution is only compatible with .NET 6 and higher because of my use of JsonNode in System.Text.Json, so I surrounded the relevant registration and implementation bits with conditional compiler flags in the shorter term. Also, I cannot get Fluxor to build on my machine as it's targeting frameworks I simply don't have installed. If you've got .NET 8 installed, clone my repo below for a .NET 8-only version of this.

A .NET 8 version of this can be seen on the latest commit here, but I'll take a moment to walk through the idea.

I built off the idea of using an action to identify when the state has initialized and an effect that triggers from that. I've added some extensions to the registration method that allows the developer to opt into the persistence feature and provide an implementation of IPersistenceManager (I wrote such an implementation in my other project using session storage here) and to allow them to register any additional dependencies here that it might require. If IPersistenceManager is never registered, Fluxor runs as it did before this implementation and that's it (with a few minor tweaks).

If it is registered, when Fluxor is activating the store instance, it first marks the store as activated, dequeues any actions and then dispatches a new StoreRehydratingAction that kicks off a registered effect that reads any persisted state from the IPersistenceManager and uses a new method on the store to rehyrdrate the state on a per-feature basis (using that RestoreState method that explicitly says it shouldn't be used for this - whoops). This concludes with a StoreRehydratedAction in case anyone wants to subscribe to it for some reason.

On a location change event in the Fluxor component, it dispatches a PersistingStoreAction and this does the opposite. It builds a JSON object (each feature to a separate named key) and persists this via the IPersistenceManager and fires off a PersistedStoreAction.

This implementation avoids having the project take on any other package dependencies and leaves it to the developer to decide where and how they want to persist any data, whether that be in session state as my demo does or bind it to a session ID and save it somewhere on the server - their choice. Just implement the IPersistenceManager and register with the provided method on FluxorOptions.

Remaining work:

  • Again, this is only compatible with .NET 6, .NET 7 and .NET 8 because of the use of System.Text.Json for building the JsonNode during the serialization and deserialization steps. I imagine the same can be done with Newtonsoft.Json, if you want to take on another dependency.
  • I see some flicker in WebAssembly where it's initializing the store, then reading the state and updating it, so this could be improved on.
  • I thought persisting it on location change is often enough (wouldn't want to do it every single action) as I don't know that Blazor jumps render mode until you actually change the page. In all my testing, it always required a page change to swap modes. If that changes in a future release, perhaps they'll give us an event to help out. But if there's some notification of this or users want to save more frequently, this approach will need to be tweaked.
  • I save to the browser's session store in my demo so that when I relaunch the app, it doesn't just load the state from the previous session. I wasn't able to find any sort of common value (for non-authenticated users) that give me some sort of identifier indicating that WASM-session X matched Server-session X (so that if X didn't match, it would trigger a clear of the store). I added a method to IPersistenceManager so this is covered in a server-side store scenario, so perhaps this can be improved on.

@GordonBeeming
Copy link

@WhitWaldo thanks for the effort in getting this working ❤️

@mrpmorris

  1. Would this fix make it into 5.9.x or only 6.0?
  2. If not, do you know roughly when you are wanting 6.0 to reach stable?

@mrpmorris
Copy link
Owner

It'll be in 6.0

I will be releasing Beta versions, but not until they are stable so should be okay to use.

@GordonBeeming
Copy link

Great stuff, thanks @mrpmorris 🙂

@WhitWaldo
Copy link
Author

I've made a few changes to the PR and also updated my .NET 8 demonstration to reflect them. As before, I can't actually build this locally as I'm missing some of these older .NET SDKs, but I copy/pasted the changed pieces from my demo, so I'm fairly confident it should work.

  • As before, this is still dependent on .NET 6 and later because of the System.Text.Json dependency. My latest work doesn't change this at all.
  • The earlier version relied on the OnAfterRenderAsync method for triggering rehydration as the demonstration showed compatibility with IJSRuntime to persist the state to the user's browser via local or session state. I've since switched to use OnInitializedAsync so the rehyrdration occurs before it's rendered, eliminating the flicker. This does mean that this cannot be used with local/session storage any longer. My demo instead simulates a server-side Redis instance via a singleton service maintaining the value.
  • The earlier version also relied on NavigationManager to be the impetus triggering the state persistence. After some experimentation with PersistentComponentState, I (re-)discovered that the render mode occurs on a per-component basis and as those aren't pages, they wouldn't necessarily trigger a location change event on the NavigationManager. While I briefly looked at using PersistentComponentState to store the Store state, it is only able to write to the state whenever the event callback is being invoked indicating a render mode change and thus isn't suitable for more frequent writes. Rather, persistence now happens much more often - any time there's at least one action in the queue that's not one of the internal Fluxor system actions (e.g. rehydration, persistence, initialization), it will add an OnPersistingStateAction to the end of the queue ensuring that regardless of the component engaging with the State, all user-supplied actions are committed.
  • I couldn't find anything in Fluxor that specifically keeps track of whether or not the Feature states are actually updated as a result of any of the actions' downstream effects - if there's such a flag, that could be another useful gate to limit unnecessary state persistence operations so it only runs when the state was actually modified (which, given Reducers, will likely happen often anyway - so the value here outside of a demo might be negligible).
  • My demo is a bit more complex to reflect the real-world scenario that context running in web assembly will need to connect remotely to the server to get/set the current state whereas the server can simply inject the service it's using. Put simply, both the server and client implement an IStateService that's registered to a separate implementation for either project. On the server, this is a singleton that could just as easily be replaced with a Redis connection. On the client, this injects an IHttpClientFactory, creates the HttpClient and calls either a get or set method on a controller exposed by the server that itself injects its local IStateService (that singleton previously discussed). All of this will need to be added by the user (but would be anyway, because of the nature of the InteractiveAuto render mode) and isn't part of the Fluxor project. Rather, Fluxor, as before, simply requires that if Persistence is enabled, something that implements IPersistenceManager must be provided during the DI registration. Here, this injects an IStateService meaning that it gets either of the above implementation depending on the render mode it's using to perform that basic get/set operation, which again, happens at OnInitializedAsync as part of the state initialization, so entirely pre-empting any flicker on the page.

At this point, this is working well enough for my purposes that I don't know any other active work is really necessary unless you really want to replace the System.Text.Json dependency for something compatible with the broader set of targets you're looking to support.

I don't see a branch that specifically identifies as the 6.0 release, but if you can point me in the right direction, I'd be happy to apply my changes to that branch as well in a separate PR.

@@ -15,9 +15,11 @@
</ItemGroup>

<ItemGroup>
<PackageReference Condition="'$(TargetFramework)' == 'net8.0'" Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="8.0.0" />
<PackageReference Condition="'$(TargetFramework)' == 'net7.0'" Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="7.0.0" />
<PackageReference Condition="'$(TargetFramework)' == 'net6.0'" Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="6.0.0" />
<PackageReference Condition="'$(TargetFramework)' == 'net5.0'" Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="5.0.0" />
<PackageReference Condition="'$(TargetFramework)' != 'net7.0' AND '$(TargetFramework)' != 'net6.0' AND '$(TargetFramework)' != 'net5.0'" Include="Microsoft.Extensions.DependencyInjection.Abstractions" Version="3.1.0" />
Copy link
Contributor

@jafin jafin Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For Microsoft.Extensions.DependencyInjection.Abstractions" Version="3.1.0 does this need to be removed or a condition added for != 'net8.0 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rather assumed that whatever was there was compatible up to the current supported build prior to my adding .NET 8 to the list - presumably that includes .NET Core 3.1 as-is if that's still a supported target, so I wouldn't think anything else is necessary.

@@ -14,6 +21,10 @@ public class Store : IStore, IActionSubscriber, IDisposable
/// <see cref="IStore.Initialized"/>
public Task Initialized => InitializedCompletionSource.Task;

#if NET6_0_OR_GREATER
private readonly IPersistenceManager? _persistenceManager;
Copy link
Contributor

@jafin jafin Apr 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider following naming convention of project.? Uppercase vs leading underscore for privates.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I defer to the maintainer's preference here.

jafin pushed a commit to jafin/Fluxor that referenced this pull request Apr 16, 2024
@mikeppcom
Copy link

mikeppcom commented Apr 23, 2024

Will this PR be in beta#3/V6? We make heavy use of Fluxor and are moving to a .NET8 hybrid Blazor server/client render model and are facing the issue that actions are no longer being dispatched.

@wave9d
Copy link

wave9d commented May 29, 2024

Will this PR be in beta#3/V6? We make heavy use of Fluxor and are moving to a .NET8 hybrid Blazor server/client render model and are facing the issue that actions are no longer being dispatched.

Any news on this PR being available for a beta release ? We're in the same situation as @mikeppcom

@mrpmorris
Copy link
Owner

try specifying the @rendermode in Routes.razor or on the <StoreInitializer compoonent

dennisreimann added a commit to btcpayserver/app that referenced this pull request Jun 3, 2024
We should be able to revert this once mrpmorris/Fluxor#459 is fixed.

More context: mrpmorris/Fluxor#481.
@orosbogdan
Copy link

try specifying the @rendermode in Routes.razor or on the <StoreInitializer compoonent

The problem would be if you wanted to have both server interactive and wasm interactive pages in your app right?
Same for auto render mode I think.

@WhitWaldo
Copy link
Author

try specifying the @rendermode in Routes.razor or on the <StoreInitializer compoonent

The problem would be if you wanted to have both server interactive and wasm interactive pages in your app right? Same for auto render mode I think.

If you don't specify an interactive mode (e.g. InteractiveServer, InteractiveWebAssembly or InteractiveAuto), you simply won't see any changes made to the page/component. If the changes are in response to interaction with an HTML element, the method isn't called and thus the value isn't updated and propagated and no changes are made to the page. For any interactivity, you have to specify one of the above interactive modes.

If your whole app already uses strictly interactive WASM or interactive server, I don't know that you'd have a problem with Fluxor's latest public build. This PR was made specifically to support InteractiveAuto as it's the transition from Server to WebAssembly (and back) that was causing issues as the state wouldn't be available across contexts.

@orosbogdan
Copy link

try specifying the @rendermode in Routes.razor or on the <StoreInitializer compoonent

The problem would be if you wanted to have both server interactive and wasm interactive pages in your app right? Same for auto render mode I think.

If you don't specify an interactive mode (e.g. InteractiveServer, InteractiveWebAssembly or InteractiveAuto), you simply won't see any changes made to the page/component. If the changes are in response to interaction with an HTML element, the method isn't called and thus the value isn't updated and propagated and no changes are made to the page. For any interactivity, you have to specify one of the above interactive modes.

If your whole app already uses strictly interactive WASM or interactive server, I don't know that you'd have a problem with Fluxor's latest public build. This PR was made specifically to support InteractiveAuto as it's the transition from Server to WebAssembly (and back) that was causing issues as the state wouldn't be available across contexts.

Would an blazor project having 2 separate pages, 1 server interactive and 1 wasm interactive work with latest fluxor build ?

@WhitWaldo
Copy link
Author

WhitWaldo commented Jun 4, 2024

try specifying the @rendermode in Routes.razor or on the <StoreInitializer compoonent

The problem would be if you wanted to have both server interactive and wasm interactive pages in your app right? Same for auto render mode I think.

If you don't specify an interactive mode (e.g. InteractiveServer, InteractiveWebAssembly or InteractiveAuto), you simply won't see any changes made to the page/component. If the changes are in response to interaction with an HTML element, the method isn't called and thus the value isn't updated and propagated and no changes are made to the page. For any interactivity, you have to specify one of the above interactive modes.
If your whole app already uses strictly interactive WASM or interactive server, I don't know that you'd have a problem with Fluxor's latest public build. This PR was made specifically to support InteractiveAuto as it's the transition from Server to WebAssembly (and back) that was causing issues as the state wouldn't be available across contexts.

Would an blazor project having 2 separate pages, 1 server interactive and 1 wasm interactive work with latest fluxor build ?

If they use separate state from each other, the latest public build should work fine. Your issue is going to if you attempt to mix state between the two because they have no mechanism to communicate it back and forth.

That's the point of this PR, is to add a mechanism to persist the state regardless of where it's being updated so that either environment can access the latest version of that state, even if any given component is shifting between the two (e.g. per InteractiveAuto it'll start running on the server and eventually run from WASM).

The idea is that the data is persisted on the server via an IPersistenceManager (my demo simulates a Redis instance by using an in-memory key/value store as a singleton service) and then implement an IStateService on both the server and wasm clients to persist and retrieve the state. On the server, this pulls from the IPersistenceManager, on the client, this implements an HttpClient to get the same from the server. This way, we ensure that the state is in sync regardless of whether the next component accessing it is doing so from the client or server.

Do note that this does make the implementation quite chatty, but I was unable to find a better way of doing it in the current release of Web Apps. This might help in .NET 9, but we'll see.

@jafin
Copy link
Contributor

jafin commented Jun 10, 2024

This didn't make it into v6?

@mikeppcom
Copy link

mikeppcom commented Jun 10, 2024 via email

@WhitWaldo
Copy link
Author

WhitWaldo commented Jun 11, 2024

Jason, There isn’t an issue with Fluxor as such. If you’re using it with a web app you need to initialize it in the routes component with a render mode. <Fluxor.Blazor.Web.StoreInitializer @rendermode="InteractiveServer" /> Thanks to Pete for pointing this out. By the way not sure that webapps are really production ready as the only way I’ve been able to get them to work with layouts is to set the render mode at a global level and switch between server and client using a hard navigation.

You're right in that Fluxor will work fine so long as your application and components run entirely in either a server (personally tested) or web assembly (untested) context. Sure, if you have components that run on WebAssembly but don't inject Fluxor state, they'll work fine too. But merely running initializing the store on the server isn't going to suddenly open up some backchannel for your WebAssembly components that take a Fluxor state dependency to communicate and keep in sync with it.

Rather, this PR adds a persistence layer using the same approach provided in the sparse guidance for using the new Web Apps and leaves it as an exercise for the developer to decide just how the state itself is persisted (on the server) and how the WebAssembly components will communicate with the server to read and write its own updates. When the component is initialized then, it queries this central store (which is accessible now regardless of where the component is running) and when it applies the batch of changes, it persists that result back (so other components running in either context can access the same).

Regarding layouts, assuming you're putting all your server code in one project and your client code in another (as the Web App template in VS does), I recommend you shift all your layouts to the client project as they'll be available in either render mode. The layouts themselves shouldn't necessarily need any interactivity, so they can remain statically rendered while remaining accessible for any pages in either render mode to utilize them. I personally maintain the global default (static SSR). For those components that need any interactivity support, I use interactive server for those components I haven't yet migrated to use an API endpoint for and interactive auto for those I have and it's worked fine.

@mrpmorris mrpmorris closed this Jun 12, 2024
@mrpmorris
Copy link
Owner

You'll need to use the workaround for now.

I intend to use a slightly different approach in V7 that should suit all the different scenarios.

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.

None yet

7 participants