-
Notifications
You must be signed in to change notification settings - Fork 10.3k
Update ResourceCollectionProvider
to take advantage of Declarative persistent component state
#61748
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
base: main
Are you sure you want to change the base?
Update ResourceCollectionProvider
to take advantage of Declarative persistent component state
#61748
Conversation
@javiercn, is it possible that we are not restoring the state for webassembly interactive at all? We have it done for server when the circuit is set up:
but I cannot find any aspnetcore/src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHost.cs Line 140 in 0508333
is the only one but it's not triggered on "rehydration" after prerendering. |
@ilonatommy not sure what's going on here, but we have tests covering this. You could try debugging the sample project to figure out what is going on |
Summary what's going on: Key in prerendering for There are 2 reasons I suspect:
Even if we skip the version by using simplified name Logs for using the simplified assembly name:
|
@ilonatommy good catch. This is something that we solved for other types (AntiforgeryStateProvider) relying on the public abstraction, but I don't think we want to add a public abstraction for this type, as it's completely internal. We should figure out what we want to do here, but there are some options like explicitly providing a key in these cases where stuff is "shared". We want to support passing information across boundaries without having to require any public API, we just need to tweak the way we do this. I think we still want to use the full name to compute the key by default as that's the common case and guarantee's us that we don't squat over types that have the same name. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the handling of persistent component state for ResourceCollection by moving its saving and restoring responsibility to PersistentServicesRegistry.
- Update ResourceCollectionProvider to use a persistent state attribute and remove internal state persistence logic.
- Register ResourceCollectionProvider for persistent state registration in both WebAssemblyHostBuilder and RazorComponentsServiceCollectionExtensions.
- Refactor key computation in PersistentServicesRegistry to ensure canonical assembly names are used.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs | Registers ResourceCollectionProvider as a singleton with an added persistent service registration. |
src/Components/Shared/src/ResourceCollectionProvider.cs | Removes internal persistent state logic and switches to a property marked with SupplyParameterFromPersistentComponentState. |
src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.cs | Updates the code to use the new property setter instead of a method call for setting the URL. |
src/Components/Endpoints/src/DependencyInjection/RazorComponentsServiceCollectionExtensions.cs | Registers ResourceCollectionProvider as a scoped service with persistent service registration. |
src/Components/Components/src/PersistentState/PersistentServicesRegistry.cs | Refactors the ComputeKey method to use a canonical mapping based on assembly names. |
Comments suppressed due to low confidence (2)
src/Components/WebAssembly/WebAssembly/src/Hosting/WebAssemblyHostBuilder.cs:310
- ResourceCollectionProvider is registered as a singleton in WebAssemblyHostBuilder, while in RazorComponentsServiceCollectionExtensions it is registered as scoped. Consider aligning the DI registration lifetimes across the project to avoid inconsistent behavior.
Services.AddSingleton<ResourceCollectionProvider>();
src/Components/Shared/src/ResourceCollectionProvider.cs:21
- [nitpick] The previous registration of a persisting callback on the ResourceCollectionUrl setter has been removed. Please ensure that the external registration via RegisterPersistentComponentStateServiceCollectionExtensions fully covers the intended persistence behavior to avoid potential state loss.
get => _url; set { ... }
// Mapping for internal classes bundled in different assemblies during prerendering and WASM rendering. | ||
private static readonly Dictionary<string, string> _canonicalMap = new(StringComparer.OrdinalIgnoreCase) | ||
{ | ||
{ "Microsoft.AspNetCore.Components.Endpoints", "Microsoft.AspNetCore.Components" }, | ||
{ "Microsoft.AspNetCore.Components.WebAssembly", "Microsoft.AspNetCore.Components" } | ||
}; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, but ideally, we don't want to do it this way. Microsoft.AspNetCore.Components shouldn't really depend on Endpoints or WebAssembly (by depend on I mean make a reference to those or include some built-in knowledge about them).
This makes it problematic the next time we add one of these things. It's "interaction at a distance" where someone will ask themselves why something doesn't work and it's because they are missing an update to this mapping.
Instead, maybe we can consider some heuristic, but I don't know exactly which one it is. We could do something like "if the type is internal, we could consider the type name only.
Follow-up for #60634.
Framework changes
ResourceCollectionUrl
saving and restoring responsibility is moved toPersistentServicesRegistry
.I order to support
[SupplyParameterFromPersistentComponentState]
attribute, the field holding resources url had to be transformed from private field to a public property (ResourceCollectionUrl
). Now it exposes a setter that replacedSetResourceCollectionUrl
method used internally on initialization.Key computation for some internal assemblies that are referenced by both: server and client blazor code was not deterministic. We produced the key hashing a string that contained assembly name. Assembly name changes between prerendering and WASM rendering in case of internal shared assemblies. As a workaround, we decided to detect types with
internal
modifier and skip assembly name in the key computation.Test changes
AddRazorComponentsTwice_DoesNotDuplicateServices
test was checking if double call ofAddRazorComponents
does not duplicate the services that were supposed to be instantiated once. Before this PRIPersistentServiceRegistration
was registered only once, so it was listed as 'single-registration' service. In this PR we added a new registration ofIPersistentServiceRegistration
in the sameAddRazorComponents
. We had to change test's expectations and listIPersistentServiceRegistration
as a 'multi-registration' service.