Skip to content

Commit 5d2bb55

Browse files
Copilotjaviercn
andcommitted
Address review feedback: Use filters directly instead of scenarios in registrations
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
1 parent 757ba83 commit 5d2bb55

File tree

5 files changed

+40
-80
lines changed

5 files changed

+40
-80
lines changed

src/Components/Components/src/PersistentComponentState.cs

Lines changed: 5 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -174,45 +174,16 @@ public RestoringComponentStateSubscription RegisterOnRestoring(
174174
ArgumentNullException.ThrowIfNull(filter);
175175
ArgumentNullException.ThrowIfNull(callback);
176176

177-
// Create a wrapper scenario that uses the filter
178-
var filterScenario = new FilterWrapperScenario(filter);
179-
var registration = new RestoreComponentStateRegistration(filterScenario, callback);
177+
var registration = new RestoreComponentStateRegistration(filter, callback);
180178
_restoringCallbacks.Add(registration);
181179

182-
// If we already have a current scenario and it matches, invoke immediately
183-
if (CurrentScenario != null && ShouldInvokeCallback(filterScenario, CurrentScenario))
180+
// If we already have a current scenario and the filter matches, invoke immediately
181+
if (CurrentScenario != null && filter.ShouldRestore(CurrentScenario))
184182
{
185183
callback();
186184
}
187185

188-
return new RestoringComponentStateSubscription(_restoringCallbacks, filterScenario, callback);
189-
}
190-
191-
/// <summary>
192-
/// A scenario wrapper that uses a filter to determine if it should match the current scenario.
193-
/// </summary>
194-
private sealed class FilterWrapperScenario : IPersistentComponentStateScenario
195-
{
196-
private readonly IPersistentStateFilter _filter;
197-
198-
public FilterWrapperScenario(IPersistentStateFilter filter)
199-
{
200-
_filter = filter;
201-
}
202-
203-
public bool IsRecurring => true; // Filter-based scenarios can be recurring
204-
205-
public bool ShouldMatchScenario(IPersistentComponentStateScenario currentScenario)
206-
{
207-
return _filter.ShouldRestore(currentScenario);
208-
}
209-
210-
public override bool Equals(object? obj)
211-
{
212-
return obj is FilterWrapperScenario other && ReferenceEquals(_filter, other._filter);
213-
}
214-
215-
public override int GetHashCode() => _filter.GetHashCode();
186+
return new RestoringComponentStateSubscription(_restoringCallbacks, filter, callback);
216187
}
217188

218189
/// <summary>
@@ -244,32 +215,13 @@ private void InvokeRestoringCallbacks(IPersistentComponentStateScenario scenario
244215
{
245216
var registration = _restoringCallbacks[i];
246217

247-
if (ShouldInvokeCallback(registration.Scenario, scenario))
218+
if (registration.Filter.ShouldRestore(scenario))
248219
{
249220
registration.Callback();
250-
251-
// Remove non-recurring callbacks after invocation
252-
if (!registration.Scenario.IsRecurring)
253-
{
254-
_restoringCallbacks.RemoveAt(i);
255-
}
256221
}
257222
}
258223
}
259224

260-
private static bool ShouldInvokeCallback(IPersistentComponentStateScenario callbackScenario, IPersistentComponentStateScenario currentScenario)
261-
{
262-
// Special handling for filter wrapper scenarios
263-
if (callbackScenario is FilterWrapperScenario filterWrapper)
264-
{
265-
return filterWrapper.ShouldMatchScenario(currentScenario);
266-
}
267-
268-
// For regular scenarios, match by type and properties
269-
return callbackScenario.GetType() == currentScenario.GetType() &&
270-
callbackScenario.Equals(currentScenario);
271-
}
272-
273225
private bool TryTake(string key, out byte[]? value)
274226
{
275227
ArgumentNullException.ThrowIfNull(key);

src/Components/Components/src/PersistentState/ComponentStatePersistenceManager.cs

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ public class ComponentStatePersistenceManager
1717
private bool _stateIsPersisted;
1818
private readonly PersistentServicesRegistry? _servicesRegistry;
1919
private readonly Dictionary<string, byte[]> _currentState = new(StringComparer.Ordinal);
20-
private int _restoreCallCount;
20+
private bool _isFirstRestore = true;
2121

2222
/// <summary>
2323
/// Initializes a new instance of <see cref="ComponentStatePersistenceManager"/>.
@@ -72,12 +72,11 @@ public async Task RestoreStateAsync(
7272
{
7373
var data = await store.GetPersistedStateAsync();
7474

75-
_restoreCallCount++;
76-
77-
if (_restoreCallCount == 1)
75+
if (_isFirstRestore)
7876
{
7977
// First-time initialization
8078
State.InitializeExistingState(data);
79+
_isFirstRestore = false;
8180
}
8281
else
8382
{

src/Components/Components/src/RestoreComponentStateRegistration.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,12 @@ namespace Microsoft.AspNetCore.Components;
88
/// </summary>
99
internal readonly struct RestoreComponentStateRegistration
1010
{
11-
public RestoreComponentStateRegistration(IPersistentComponentStateScenario scenario, Action callback)
11+
public RestoreComponentStateRegistration(IPersistentStateFilter filter, Action callback)
1212
{
13-
Scenario = scenario;
13+
Filter = filter;
1414
Callback = callback;
1515
}
1616

17-
public IPersistentComponentStateScenario Scenario { get; }
17+
public IPersistentStateFilter Filter { get; }
1818
public Action Callback { get; }
1919
}

src/Components/Components/src/RestoringComponentStateSubscription.cs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,28 +9,28 @@ namespace Microsoft.AspNetCore.Components;
99
public readonly struct RestoringComponentStateSubscription : IDisposable
1010
{
1111
private readonly List<RestoreComponentStateRegistration>? _callbacks;
12-
private readonly IPersistentComponentStateScenario? _scenario;
12+
private readonly IPersistentStateFilter? _filter;
1313
private readonly Action? _callback;
1414

1515
internal RestoringComponentStateSubscription(
1616
List<RestoreComponentStateRegistration> callbacks,
17-
IPersistentComponentStateScenario scenario,
17+
IPersistentStateFilter filter,
1818
Action callback)
1919
{
2020
_callbacks = callbacks;
21-
_scenario = scenario;
21+
_filter = filter;
2222
_callback = callback;
2323
}
2424

2525
/// <inheritdoc />
2626
public void Dispose()
2727
{
28-
if (_callbacks != null && _scenario != null && _callback != null)
28+
if (_callbacks != null && _filter != null && _callback != null)
2929
{
3030
for (int i = _callbacks.Count - 1; i >= 0; i--)
3131
{
3232
var registration = _callbacks[i];
33-
if (ReferenceEquals(registration.Scenario, _scenario) && ReferenceEquals(registration.Callback, _callback))
33+
if (ReferenceEquals(registration.Filter, _filter) && ReferenceEquals(registration.Callback, _callback))
3434
{
3535
_callbacks.RemoveAt(i);
3636
break;

src/Components/Components/src/SupplyParameterFromPersistentComponentStateValueProvider.cs

Lines changed: 24 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ internal sealed class SupplyParameterFromPersistentComponentStateValueProvider(P
2121
private static readonly ConcurrentDictionary<(Type, string), PropertyGetter> _propertyGetterCache = new();
2222

2323
private readonly Dictionary<ComponentState, PersistingComponentStateSubscription> _subscriptions = [];
24+
private readonly Dictionary<(ComponentState, string), object?> _scenarioRestoredValues = [];
2425

2526
public bool IsFixed => false;
2627
// For testing purposes only
@@ -40,8 +41,15 @@ public bool CanSupplyValue(in CascadingParameterInfo parameterInfo)
4041
public object? GetCurrentValue(object? key, in CascadingParameterInfo parameterInfo)
4142
{
4243
var componentState = (ComponentState)key!;
44+
45+
// Check if we have a scenario-restored value first
46+
var valueKey = (componentState, parameterInfo.PropertyName);
47+
if (_scenarioRestoredValues.TryGetValue(valueKey, out var scenarioValue))
48+
{
49+
return scenarioValue;
50+
}
51+
4352
var storageKey = ComputeKey(componentState, parameterInfo.PropertyName);
44-
4553
return state.TryTakeFromJson(storageKey, parameterInfo.PropertyType, out var value) ? value : null;
4654
}
4755

@@ -293,27 +301,28 @@ private void RegisterScenarioRestorationCallback(ComponentState subscriber, in C
293301
{
294302
// Check for IPersistentStateFilter attributes
295303
var filterAttributes = propertyInfo.GetCustomAttributes(typeof(IPersistentStateFilter), inherit: true);
296-
if (filterAttributes.Length == 0)
304+
305+
// Register restoration callbacks for each filter
306+
foreach (IPersistentStateFilter filter in filterAttributes)
297307
{
298-
return; // No filters, no scenario-based restoration needed
308+
RegisterRestorationCallback(subscriber, parameterInfo, filter);
299309
}
310+
}
300311

312+
[UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access otherwise can break functionality when trimming application code", Justification = "Property types of rendered components are preserved through other means and won't get trimmed.")]
313+
[UnconditionalSuppressMessage("Trimming", "IL2072:'type' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicConstructors', 'DynamicallyAccessedMemberTypes.PublicFields', 'DynamicallyAccessedMemberTypes.PublicProperties' in call to target method. The return value of the source method does not have matching annotations.", Justification = "Property types of rendered components are preserved through other means and won't get trimmed.")]
314+
private void RegisterRestorationCallback(ComponentState subscriber, in CascadingParameterInfo parameterInfo, IPersistentStateFilter filter)
315+
{
301316
var storageKey = ComputeKey(subscriber, parameterInfo.PropertyName);
302317
var propertyType = parameterInfo.PropertyType;
303-
var component = subscriber.Component;
318+
var valueKey = (subscriber, parameterInfo.PropertyName);
304319

305-
// Register restoration callbacks for each filter
306-
foreach (IPersistentStateFilter filter in filterAttributes)
320+
state.RegisterOnRestoring(filter, () =>
307321
{
308-
state.RegisterOnRestoring(filter, () =>
322+
if (state.TryTakeFromJson(storageKey, propertyType, out var value))
309323
{
310-
if (state.TryTakeFromJson(storageKey, propertyType, out var value))
311-
{
312-
// Set the property value on the component
313-
propertyInfo.SetValue(component, value);
314-
// The component will re-render naturally when needed
315-
}
316-
});
317-
}
324+
_scenarioRestoredValues[valueKey] = value;
325+
}
326+
});
318327
}
319328
}

0 commit comments

Comments
 (0)