-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Fix Blazor persistent component state restoration for components without keys and add E2E test coverage #63194
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?
Conversation
Greetings human! You've submitted a PR that modifies code that is shared with https://github.com/dotnet/runtime . Please make sure you synchronize this code with the changes in that repo! |
Co-authored-by: javiercn <[email protected]>
Co-authored-by: javiercn <[email protected]>
// Assert - Should return the restored value, not the component's property value | ||
Assert.Equal("restored-value", firstResult); | ||
|
||
// Simulate component property being updated after restoration | ||
component.State = "updated-component-value"; | ||
|
||
// Second call should return the updated component value since the flag was reset | ||
var secondResult = subscription.GetOrComputeLastValue(); | ||
Assert.Equal("updated-component-value", secondResult); | ||
|
||
subscription.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void RestoreProperty_WorksCorrectly_ForComponentsWithoutKey() | ||
{ | ||
// Arrange - This test simulates components being added/removed without @key | ||
var initialState = new Dictionary<string, byte[]>(); | ||
var state = new PersistentComponentState(initialState, [], []); | ||
var renderer = new TestRenderer(); | ||
|
||
// First component instance | ||
var component1 = new TestComponent { State = "initial-value-1" }; | ||
var componentState1 = CreateComponentState(renderer, component1, null, null); | ||
|
||
var key = PersistentStateValueProviderKeyResolver.ComputeKey(componentState1, nameof(TestComponent.State)); | ||
initialState[key] = JsonSerializer.SerializeToUtf8Bytes("persisted-value-from-previous-session", JsonSerializerOptions.Web); | ||
state.InitializeExistingState(initialState, RestoreContext.LastSnapshot); | ||
|
||
var cascadingParameterInfo = CreateCascadingParameterInfo(nameof(TestComponent.State), typeof(string)); | ||
var serviceProvider = new ServiceCollection().BuildServiceProvider(); | ||
var logger = NullLogger.Instance; | ||
|
||
var subscription1 = new PersistentValueProviderComponentSubscription( | ||
state, componentState1, cascadingParameterInfo, serviceProvider, logger); | ||
|
||
// Act - Simulate component being destroyed and recreated (like during navigation) | ||
var result1 = subscription1.GetOrComputeLastValue(); | ||
subscription1.Dispose(); | ||
|
||
// Simulate a new component instance being created (like after navigation back) | ||
var component2 = new TestComponent { State = "initial-value-2" }; | ||
var componentState2 = CreateComponentState(renderer, component2, null, null); | ||
|
||
// Re-populate state for the new component instance | ||
initialState[key] = JsonSerializer.SerializeToUtf8Bytes("persisted-value-from-previous-session", JsonSerializerOptions.Web); | ||
state.InitializeExistingState(initialState, RestoreContext.LastSnapshot); | ||
|
||
var subscription2 = new PersistentValueProviderComponentSubscription( | ||
state, componentState2, cascadingParameterInfo, serviceProvider, logger); | ||
|
||
var result2 = subscription2.GetOrComputeLastValue(); | ||
|
||
// Assert - Both instances should restore the persisted value correctly | ||
Assert.Equal("persisted-value-from-previous-session", result1); | ||
Assert.Equal("persisted-value-from-previous-session", result2); | ||
|
||
subscription2.Dispose(); | ||
} | ||
|
||
[Fact] | ||
public void RestoreProperty_WithSkipNotifications_StillSetsIgnoreComponentPropertyValue() | ||
{ | ||
// This test verifies that the fix works even when skipNotifications is true, | ||
// which is the scenario that was broken before our fix | ||
|
||
// Arrange | ||
var initialState = new Dictionary<string, byte[]>(); | ||
var state = new PersistentComponentState(initialState, [], []); | ||
var renderer = new TestRenderer(); | ||
var component = new TestComponent { State = "component-value" }; | ||
var componentState = CreateComponentState(renderer, component, null, null); | ||
|
||
var key = PersistentStateValueProviderKeyResolver.ComputeKey(componentState, nameof(TestComponent.State)); | ||
initialState[key] = JsonSerializer.SerializeToUtf8Bytes("persisted-value", JsonSerializerOptions.Web); | ||
state.InitializeExistingState(initialState, RestoreContext.LastSnapshot); | ||
|
||
var cascadingParameterInfo = CreateCascadingParameterInfo(nameof(TestComponent.State), typeof(string)); | ||
var serviceProvider = new ServiceCollection().BuildServiceProvider(); | ||
var logger = NullLogger.Instance; | ||
|
||
var subscription = new PersistentValueProviderComponentSubscription( | ||
state, componentState, cascadingParameterInfo, serviceProvider, logger); | ||
|
||
// Mark the subscription as having pending initial value to trigger skipNotifications = true | ||
var pendingField = typeof(PersistentValueProviderComponentSubscription) | ||
.GetField("_hasPendingInitialValue", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); | ||
pendingField.SetValue(subscription, true); | ||
|
||
// Act - Call RestoreProperty which should skipNotifications but still set _ignoreComponentPropertyValue | ||
var restoreMethod = typeof(PersistentValueProviderComponentSubscription) | ||
.GetMethod("RestoreProperty", System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); | ||
restoreMethod.Invoke(subscription, null); | ||
|
||
// Assert - Even with skipNotifications = true, the next GetOrComputeLastValue should return the restored value | ||
var result = subscription.GetOrComputeLastValue(); | ||
Assert.Equal("persisted-value", result); | ||
|
||
subscription.Dispose(); | ||
} |
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.
[Fact]
public async Task GetOrComputeLastValue_FollowsCorrectValueTransitionSequence()
{
// Arrange
var appState = new Dictionary<string, byte[]>();
var manager = new ComponentStatePersistenceManager(NullLogger<ComponentStatePersistenceManager>.Instance);
var state = manager.State;
var serviceProvider = PersistentStateProviderServiceCollectionExtensions.AddSupplyValueFromPersistentComponentStateProvider(new ServiceCollection())
.AddSingleton(manager)
.AddSingleton(manager.State)
.AddFakeLogging()
.BuildServiceProvider();
var renderer = new TestRenderer(serviceProvider);
var provider = (PersistentStateValueProvider)renderer.ServiceProviderCascadingValueSuppliers.Single();
var component = new TestComponent { State = "initial-property-value" };
var componentId = renderer.AssignRootComponentId(component);
var componentState = renderer.GetComponentState(component);
// Pre-populate the state with serialized data
var key = PersistentStateValueProviderKeyResolver.ComputeKey(componentState, nameof(TestComponent.State));
appState[key] = JsonSerializer.SerializeToUtf8Bytes("first-restored-value", JsonSerializerOptions.Web);
await manager.RestoreStateAsync(new TestStore(appState), RestoreContext.InitialValue);
await renderer.Dispatcher.InvokeAsync(() => renderer.RenderRootComponentAsync(componentId, ParameterView.Empty));
var cascadingParameterInfo = CreateCascadingParameterInfo(nameof(TestComponent.State), typeof(string));
// Act & Assert - First call: Returns restored value from state
Assert.Equal("first-restored-value", component.State);
// Change the component's property value
component.State = "updated-property-value";
// Second call: Returns the component's property value
var result2 = provider.GetCurrentValue(componentState, cascadingParameterInfo);
Assert.Equal("updated-property-value", result2);
appState.Clear();
var newState = new Dictionary<string, byte[]>
{
[key] = JsonSerializer.SerializeToUtf8Bytes("second-restored-value", JsonSerializerOptions.Web)
};
// Simulate invoking the callback with a value update.
await renderer.Dispatcher.InvokeAsync(() => manager.RestoreStateAsync(new TestStore(newState), RestoreContext.ValueUpdate));
Assert.Equal("second-restored-value", component.State);
component.State = "another-updated-value";
// Other calls: Returns the updated value from state
Assert.Equal("another-updated-value", provider.GetCurrentValue(componentState, cascadingParameterInfo));
component.State = "final-updated-value";
Assert.Equal("final-updated-value", provider.GetCurrentValue(componentState, cascadingParameterInfo));
Assert.Equal("final-updated-value", provider.GetCurrentValue(componentState, cascadingParameterInfo));
}
```
You must write the tests following this approach, but note that the main difference here is that the subscription will be re-created (because it will be a new component) in between each state restoration
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.
I've rewritten the tests following your guidance to properly simulate component recreation scenarios where subscriptions are re-created between state restorations. The new tests use the full renderer setup like GetOrComputeLastValue_FollowsCorrectValueTransitionSequence()
and properly simulate:
- Component creation with initial state restoration
- Component destruction using
RemoveRootComponent()
- Component recreation with new subscription instances
- Verification that restored values are preserved correctly across these recreations
The tests now accurately reflect the real-world scenarios where components are destroyed and recreated during navigation or when components don't use @key
. Fixed in f54388f.
…ion scenarios Co-authored-by: javiercn <[email protected]>
We should look at the E2E test cases in #62824 to add also support for these other two scenarios. Specifically src/Components/test/E2ETest/Tests/StatePersistenceTest.cs and src/Components/test/testassets/Components.TestServer/RazorComponents/Pages/PersistentState/PageWithDeclarativeEnhancedNavigationPersistentComponents.razor Our test could be designed to use a query string to choose to render or not render the components. There would be one component without a key that renders always and a component that gets added/removed depending on the value of the query string. In both cases the state should be restored successfully |
…tent state restoration Co-authored-by: javiercn <[email protected]>
I've added E2E test support for the two scenarios you mentioned:
The page follows the same pattern as the existing declarative enhanced navigation page, with links in PageWithoutComponents.razor for navigation. Both scenarios should validate that persistent state restoration works correctly regardless of when components are destroyed and recreated. Implemented in 6b82560. |
…ability Co-authored-by: javiercn <[email protected]>
The
_ignoreComponentPropertyValue
flag inPersistentValueProviderComponentSubscription
was only being set conditionally withinif (!skipNotifications)
blocks, causing restored values to be ignored when components are destroyed and re-added during navigation or when components don't use@key
.Problem
When a component is added, removed, then added again (for example, during page navigation or when a component doesn't use
@key
), the restored value from persistent state storage was being ignored. This happened because:skipNotifications
is oftentrue
_ignoreComponentPropertyValue = true
assignment was inside theif (!skipNotifications)
blockGetOrComputeLastValue()
would return the component's current property value instead of the restored valueSolution
Moved
_ignoreComponentPropertyValue = true;
outside the conditional blocks so it's set unconditionally whenever we successfully restore from persistent state. This ensures that the restored value always takes precedence over the component's initial property value, regardless of notification settings.Before:
After:
Test Coverage
Added comprehensive test coverage including:
Unit Tests
_ignoreComponentPropertyValue
flag behaviorE2E Tests
Following feedback from @javiercn, added E2E test infrastructure for two critical scenarios:
PageWithConditionalPersistentComponents.razor - New test page supporting:
StatePersistenceTest.cs - New E2E tests covering:
CanRestoreStateForComponentsWithoutKeysAndConditionalRendering
- Validates both components restore state correctlyCanRestoreStateAfterConditionalComponentToggling
- Tests state restoration after component togglingThe E2E tests use query string parameters to control component rendering and validate that persistent state works correctly in all scenarios where components are dynamically added/removed, particularly during enhanced navigation in Blazor applications.
Fixes #63193.
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.