Address PR review comments - remove overloads, use null instead of default reason, implement singleton pattern

Co-authored-by: javiercn <[email protected]>

Author: Copilot

src/Components/Components/src/DefaultPersistenceReason.cs

@@ -6,11 +6,11 @@ namespace Microsoft.AspNetCore.Components;
 internal readonly struct PersistComponentStateRegistration(
     Func<Task> callback,
     IComponentRenderMode? renderMode,
-    IReadOnlyList<IPersistenceReasonFilter>? reasonFilters = null)
+    IReadOnlyList<IPersistenceReasonFilter> reasonFilters)
 {
     public Func<Task> Callback { get; } = callback;
 
     public IComponentRenderMode? RenderMode { get; } = renderMode;
 
-    public IReadOnlyList<IPersistenceReasonFilter>? ReasonFilters { get; } = reasonFilters;
+    public IReadOnlyList<IPersistenceReasonFilter> ReasonFilters { get; } = reasonFilters ?? Array.Empty<IPersistenceReasonFilter>();
 }

src/Components/Components/src/PersistComponentStateRegistration.cs

@@ -43,7 +43,7 @@ internal void InitializeExistingState(IDictionary<string, byte[]> existingState)
     /// <param name="callback">The callback to invoke when the application is being paused.</param>
     /// <returns>A subscription that can be used to unregister the callback when disposed.</returns>
     public PersistingComponentStateSubscription RegisterOnPersisting(Func<Task> callback)
-        => RegisterOnPersisting(callback, null);
+        => RegisterOnPersisting(callback, null, Array.Empty<IPersistenceReasonFilter>());
 
     /// <summary>
     /// Register a callback to persist the component state when the application is about to be paused.
@@ -53,7 +53,7 @@ public PersistingComponentStateSubscription RegisterOnPersisting(Func<Task> call
     /// <param name="renderMode"></param>
     /// <param name="reasonFilters">Filters to control when the callback should be invoked based on the persistence reason.</param>
     /// <returns>A subscription that can be used to unregister the callback when disposed.</returns>
-    public PersistingComponentStateSubscription RegisterOnPersisting(Func<Task> callback, IComponentRenderMode? renderMode, IReadOnlyList<IPersistenceReasonFilter>? reasonFilters)
+    public PersistingComponentStateSubscription RegisterOnPersisting(Func<Task> callback, IComponentRenderMode? renderMode, IReadOnlyList<IPersistenceReasonFilter> reasonFilters)
     {
         ArgumentNullException.ThrowIfNull(callback);
 
@@ -85,7 +85,7 @@ public PersistingComponentStateSubscription RegisterOnPersisting(Func<Task> call
             throw new InvalidOperationException("Registering a callback while persisting state is not allowed.");
         }
 
-        var persistenceCallback = new PersistComponentStateRegistration(callback, renderMode);
+        var persistenceCallback = new PersistComponentStateRegistration(callback, renderMode, Array.Empty<IPersistenceReasonFilter>());
 
         _registeredCallbacks.Add(persistenceCallback);
 

src/Components/Components/src/PersistentComponentState.cs

@@ -61,23 +61,14 @@ public async Task RestoreStateAsync(IPersistentComponentStateStore store)
         _servicesRegistry?.Restore(State);
     }
 
-    /// <summary>
-    /// Persists the component application state into the given <see cref="IPersistentComponentStateStore"/>.
-    /// </summary>
-    /// <param name="store">The <see cref="IPersistentComponentStateStore"/> to restore the application state from.</param>
-    /// <param name="renderer">The <see cref="Renderer"/> that components are being rendered.</param>
-    /// <returns>A <see cref="Task"/> that will complete when the state has been restored.</returns>
-    public Task PersistStateAsync(IPersistentComponentStateStore store, Renderer renderer)
-        => PersistStateAsync(store, renderer, DefaultPersistenceReason.Instance);
-
     /// <summary>
     /// Persists the component application state into the given <see cref="IPersistentComponentStateStore"/>.
     /// </summary>
     /// <param name="store">The <see cref="IPersistentComponentStateStore"/> to restore the application state from.</param>
     /// <param name="renderer">The <see cref="Renderer"/> that components are being rendered.</param>
     /// <param name="persistenceReason">The reason for persisting the state.</param>
     /// <returns>A <see cref="Task"/> that will complete when the state has been restored.</returns>
-    public Task PersistStateAsync(IPersistentComponentStateStore store, Renderer renderer, IPersistenceReason persistenceReason)
+    public Task PersistStateAsync(IPersistentComponentStateStore store, Renderer renderer, IPersistenceReason? persistenceReason = null)
     {
         if (_stateIsPersisted)
         {
@@ -186,10 +177,7 @@ private void InferRenderModes(Renderer renderer)
         }
     }
 
-    internal Task<bool> TryPauseAsync(IPersistentComponentStateStore store)
-        => TryPauseAsync(store, DefaultPersistenceReason.Instance);
-
-    internal Task<bool> TryPauseAsync(IPersistentComponentStateStore store, IPersistenceReason persistenceReason)
+    internal Task<bool> TryPauseAsync(IPersistentComponentStateStore store, IPersistenceReason? persistenceReason = null)
     {
         List<Task<bool>>? pendingCallbackTasks = null;
 
@@ -213,21 +201,21 @@ internal Task<bool> TryPauseAsync(IPersistentComponentStateStore store, IPersist
             }
 
             // Evaluate reason filters to determine if the callback should be executed for this persistence reason
-            if (registration.ReasonFilters != null)
+            if (registration.ReasonFilters.Count > 0)
             {
                 var shouldPersist = EvaluateReasonFilters(registration.ReasonFilters, persistenceReason);
                 if (shouldPersist.HasValue && !shouldPersist.Value)
                 {
                     // Filters explicitly indicate not to persist for this reason
                     continue;
                 }
-                else if (!shouldPersist.HasValue && !persistenceReason.PersistByDefault)
+                else if (!shouldPersist.HasValue && !(persistenceReason?.PersistByDefault ?? true))
                 {
                     // No filter matched and default is not to persist
                     continue;
                 }
             }
-            else if (!persistenceReason.PersistByDefault)
+            else if (!(persistenceReason?.PersistByDefault ?? true))
             {
                 // No filters defined and default is not to persist
                 continue;
@@ -306,8 +294,14 @@ static async Task<bool> AnyTaskFailed(List<Task<bool>> pendingCallbackTasks)
         }
     }
 
-    private static bool? EvaluateReasonFilters(IReadOnlyList<IPersistenceReasonFilter> reasonFilters, IPersistenceReason persistenceReason)
+    private static bool? EvaluateReasonFilters(IReadOnlyList<IPersistenceReasonFilter> reasonFilters, IPersistenceReason? persistenceReason)
     {
+        if (persistenceReason is null)
+        {
+            // No reason provided, can't evaluate filters
+            return null;
+        }
+        
         foreach (var reasonFilter in reasonFilters)
         {
             var shouldPersist = reasonFilter.ShouldPersist(persistenceReason);

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

@@ -167,7 +167,7 @@ Microsoft.AspNetCore.Components.IHandleEvent
 Microsoft.AspNetCore.Components.IHandleEvent.HandleEventAsync(Microsoft.AspNetCore.Components.EventCallbackWorkItem item, object? arg) -> System.Threading.Tasks.Task!
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.ComponentStatePersistenceManager(Microsoft.Extensions.Logging.ILogger<Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager!>! logger) -> void
-Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.PersistStateAsync(Microsoft.AspNetCore.Components.IPersistentComponentStateStore! store, Microsoft.AspNetCore.Components.RenderTree.Renderer! renderer) -> System.Threading.Tasks.Task!
+Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.PersistStateAsync(Microsoft.AspNetCore.Components.IPersistentComponentStateStore! store, Microsoft.AspNetCore.Components.RenderTree.Renderer! renderer, Microsoft.AspNetCore.Components.IPersistenceReason? persistenceReason = null) -> System.Threading.Tasks.Task!
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.RestoreStateAsync(Microsoft.AspNetCore.Components.IPersistentComponentStateStore! store) -> System.Threading.Tasks.Task!
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.State.get -> Microsoft.AspNetCore.Components.PersistentComponentState!
 Microsoft.AspNetCore.Components.InjectAttribute

src/Components/Components/src/PublicAPI.Shipped.txt

@@ -10,7 +10,6 @@ Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs.NotFoundEventArgs(stri
 Microsoft.AspNetCore.Components.Routing.NotFoundEventArgs.Path.get -> string!
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.ComponentStatePersistenceManager(Microsoft.Extensions.Logging.ILogger<Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager!>! logger, System.IServiceProvider! serviceProvider) -> void
 Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.SetPlatformRenderMode(Microsoft.AspNetCore.Components.IComponentRenderMode! renderMode) -> void
-Microsoft.AspNetCore.Components.Infrastructure.ComponentStatePersistenceManager.PersistStateAsync(Microsoft.AspNetCore.Components.IPersistentComponentStateStore! store, Microsoft.AspNetCore.Components.RenderTree.Renderer! renderer, Microsoft.AspNetCore.Components.IPersistenceReason! persistenceReason) -> System.Threading.Tasks.Task!
 Microsoft.AspNetCore.Components.Infrastructure.RegisterPersistentComponentStateServiceCollectionExtensions
 Microsoft.AspNetCore.Components.SupplyParameterFromPersistentComponentStateAttribute
 Microsoft.AspNetCore.Components.SupplyParameterFromPersistentComponentStateAttribute.SupplyParameterFromPersistentComponentStateAttribute() -> void
@@ -21,7 +20,7 @@ Microsoft.AspNetCore.Components.IPersistenceReasonFilter.ShouldPersist(Microsoft
 Microsoft.AspNetCore.Components.PersistReasonFilter<TReason>
 Microsoft.AspNetCore.Components.PersistReasonFilter<TReason>.PersistReasonFilter(bool persist) -> void
 Microsoft.AspNetCore.Components.PersistReasonFilter<TReason>.ShouldPersist(Microsoft.AspNetCore.Components.IPersistenceReason! reason) -> bool?
-Microsoft.AspNetCore.Components.PersistentComponentState.RegisterOnPersisting(System.Func<System.Threading.Tasks.Task!>! callback, Microsoft.AspNetCore.Components.IComponentRenderMode? renderMode, System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.IPersistenceReasonFilter!>? reasonFilters) -> Microsoft.AspNetCore.Components.PersistingComponentStateSubscription
+Microsoft.AspNetCore.Components.PersistentComponentState.RegisterOnPersisting(System.Func<System.Threading.Tasks.Task!>! callback, Microsoft.AspNetCore.Components.IComponentRenderMode? renderMode, System.Collections.Generic.IReadOnlyList<Microsoft.AspNetCore.Components.IPersistenceReasonFilter!>! reasonFilters) -> Microsoft.AspNetCore.Components.PersistingComponentStateSubscription
 Microsoft.Extensions.DependencyInjection.SupplyParameterFromPersistentComponentStateProviderServiceCollectionExtensions
 static Microsoft.AspNetCore.Components.Infrastructure.RegisterPersistentComponentStateServiceCollectionExtensions.AddPersistentServiceRegistration<TService>(Microsoft.Extensions.DependencyInjection.IServiceCollection! services, Microsoft.AspNetCore.Components.IComponentRenderMode! componentRenderMode) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!
 static Microsoft.AspNetCore.Components.Infrastructure.ComponentsMetricsServiceCollectionExtensions.AddComponentsMetrics(Microsoft.Extensions.DependencyInjection.IServiceCollection! services) -> Microsoft.Extensions.DependencyInjection.IServiceCollection!

src/Components/Components/src/PublicAPI.Unshipped.txt

@@ -51,8 +51,8 @@ public async ValueTask<IHtmlContent> PrerenderPersistedStateAsync(HttpContext ht
         if (store != null)
         {
             IPersistenceReason persistenceReason = IsProgressivelyEnhancedNavigation(httpContext.Request)
-                ? new PersistOnEnhancedNavigation()
-                : new PersistOnPrerendering();
+                ? PersistOnEnhancedNavigation.Instance
+                : PersistOnPrerendering.Instance;
             await manager.PersistStateAsync(store, this, persistenceReason);
             return store switch
             {
@@ -84,8 +84,8 @@ public async ValueTask<IHtmlContent> PrerenderPersistedStateAsync(HttpContext ht
             store = new CompositeStore(server, auto, webAssembly);
 
             IPersistenceReason persistenceReason = IsProgressivelyEnhancedNavigation(httpContext.Request)
-                ? new PersistOnEnhancedNavigation()
-                : new PersistOnPrerendering();
+                ? PersistOnEnhancedNavigation.Instance
+                : PersistOnPrerendering.Instance;
             await manager.PersistStateAsync(store, this, persistenceReason);
 
             foreach (var kvp in auto.Saved)

src/Components/Endpoints/src/Rendering/EndpointHtmlRenderer.PrerenderingState.cs

@@ -28,7 +28,7 @@ public async Task PauseCircuitAsync(CircuitHost circuit, bool saveStateToClient
             collector.PersistRootComponents,
             RenderMode.InteractiveServer);
 
-        await persistenceManager.PersistStateAsync(collector, renderer, new PersistOnCircuitPause());
+        await persistenceManager.PersistStateAsync(collector, renderer, PersistOnCircuitPause.Instance);
 
         if (saveStateToClient)
         {

src/Components/Server/src/Circuits/CircuitPersistenceManager.cs

@@ -0,0 +1,20 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Components.Web;
+
+/// <summary>
+/// Represents persistence when a circuit is paused.
+/// </summary>
+public sealed class PersistOnCircuitPause : IPersistenceReason
+{
+    /// <summary>
+    /// Gets the singleton instance of <see cref="PersistOnCircuitPause"/>.
+    /// </summary>
+    public static readonly PersistOnCircuitPause Instance = new();
+
+    private PersistOnCircuitPause() { }
+
+    /// <inheritdoc />
+    public bool PersistByDefault => true;
+}

src/Components/Web/src/PersistOnCircuitPause.cs

@@ -0,0 +1,20 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Microsoft.AspNetCore.Components.Web;
+
+/// <summary>
+/// Represents persistence during enhanced navigation.
+/// </summary>
+public sealed class PersistOnEnhancedNavigation : IPersistenceReason
+{
+    /// <summary>
+    /// Gets the singleton instance of <see cref="PersistOnEnhancedNavigation"/>.
+    /// </summary>
+    public static readonly PersistOnEnhancedNavigation Instance = new();
+
+    private PersistOnEnhancedNavigation() { }
+
+    /// <inheritdoc />
+    public bool PersistByDefault => false;
+}