-
Notifications
You must be signed in to change notification settings - Fork 10.4k
[Blazor] Add IPersistentComponentStateSerializer<T> interface for custom serialization extensibility #62559
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
…bility Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
…tionality Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
…ementation Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// <param name="instance">The instance to persist.</param> | ||
/// <param name="serializer">The custom serializer to use for serialization.</param> | ||
/// <param name="cancellationToken">A cancellation token that can be used to cancel the serialization operation.</param> | ||
public async Task PersistAsync<TValue>(string key, TValue instance, IPersistentComponentStateSerializer<TValue> serializer, CancellationToken cancellationToken = default) |
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.
Make this internal
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.
Made this method internal as requested, along with the corresponding TryTake method for consistency. Updated reflection calls in PersistentStateValueProvider to use NonPublic binding flags. Commit 147b5bc
/// <param name="writer">The buffer writer to write the serialized data to.</param> | ||
/// <param name="cancellationToken">A cancellation token that can be used to cancel the serialization operation.</param> | ||
/// <returns>A task that represents the asynchronous serialization operation.</returns> | ||
Task PersistAsync(T value, IBufferWriter<byte> writer, CancellationToken cancellationToken); |
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.
No cancellation token
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.
Removed the cancellation token parameter from the PersistAsync method as requested. Updated both the interface definition and all related implementations. Commit d7ed4d9
/// <param name="serializer">The custom serializer to use for deserialization.</param> | ||
/// <param name="instance">The persisted instance.</param> | ||
/// <returns><c>true</c> if the state was found; <c>false</c> otherwise.</returns> | ||
public bool TryTake<TValue>(string key, IPersistentComponentStateSerializer<TValue> serializer, [MaybeNullWhen(false)] out TValue instance) |
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.
Make this API internal
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.
Made the TryTake API internal as requested. This was already done in the previous commit but confirmed it's correct. The method is now properly scoped to internal access only. Commit d7ed4d9
var serializerType = typeof(IPersistentComponentStateSerializer<>).MakeGenericType(propertyType); | ||
var customSerializer = serviceProvider.GetService(serializerType); | ||
|
||
if (customSerializer != null) | ||
{ | ||
// Use reflection to call the generic PersistAsync method with the custom serializer | ||
var persistMethod = typeof(PersistentComponentState).GetMethod(nameof(PersistentComponentState.PersistAsync), BindingFlags.Instance | BindingFlags.Public, [typeof(string), propertyType, serializerType, typeof(CancellationToken)]); | ||
if (persistMethod != null) | ||
{ | ||
var task = (Task)persistMethod.Invoke(state, [storageKey, property, customSerializer, CancellationToken.None])!; | ||
await task; | ||
return; | ||
} | ||
} |
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.
Cache the serializers by type on a ConcurrentDictionary<Type, IPersistentComponentStateSerializer>
(see how it's done in this file.
IPersistentComponentStateSerializer should be an internal interface, have the same methods as IPersistentComponentStateSerializer but take the Type type
argument as a first argument.
IPersistentComponentStateSerializer<T>
should implement IPersistentComponentStateSerializer
with a default interface implementation that calls into Persist no matter the Type t passed as first argument
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.
Move this code outside of the lambda.
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.
Implemented the requested architecture changes:
- Added ConcurrentDictionary<Type, IPersistentComponentStateSerializer> for serializer caching (made instance-based to avoid cross-test contamination)
- Created internal IPersistentComponentStateSerializer base interface with Type parameter
- Made IPersistentComponentStateSerializer implement the base interface with default implementations
- Moved serializer resolution code outside of lambda for better performance
- Used non-generic interface methods to avoid reflection
Commit d7ed4d9
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
…eedback - Remove CancellationToken from IPersistentComponentStateSerializer interface - Create internal IPersistentComponentStateSerializer base interface with Type parameter - Add default interface implementations for type safety - Add serializer caching with ConcurrentDictionary - Move serializer resolution outside of lambda for better performance - Add PersistAsBytes and TryTakeBytes methods for raw byte operations - Update PublicAPI to reflect interface changes Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
…mination - Changed _serializerCache from static to instance field to prevent different service providers from sharing cached serializers - This ensures each PersistentStateValueProvider instance has its own serializer cache Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// <summary> | ||
/// Provides custom serialization logic for persistent component state values. | ||
/// </summary> | ||
public interface IPersistentComponentStateSerializer |
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.
Make this interface internal
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.
Made the IPersistentComponentStateSerializer interface internal as requested. Also updated the generic interface IPersistentComponentStateSerializer to internal to maintain consistent accessibility and removed the interfaces from PublicAPI.Unshipped.txt since they are no longer part of the public API. Commit 358f9e4
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.
No. IPersistentComponentStateSerializer
should be internal, and IPersistentComponentStateSerializer<T>
should be public. Also move it to a separate file.
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.
Made IPersistentComponentStateSerializer internal and moved IPersistentComponentStateSerializer to a separate file (IPersistentComponentStateSerializerOfT.cs) while keeping it public. Added adapter pattern in PersistentStateValueProvider to bridge between the interfaces. Updated PublicAPI.Unshipped.txt accordingly. Commit c1fefc5
…review feedback Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// Provides custom serialization logic for persistent component state values of type <typeparamref name="T"/>. | ||
/// </summary> | ||
/// <typeparam name="T">The type of the value to serialize.</typeparam> | ||
internal interface IPersistentComponentStateSerializer<T> : IPersistentComponentStateSerializer |
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.
IPersistentComponentStateSerializer should be public, but IPersistentComponentStateSerializer should remain internal
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.
Made IPersistentComponentStateSerializer<T> public while keeping the base interface IPersistentComponentStateSerializer internal as requested. Also added the public interface to PublicAPI.Unshipped.txt. Commit 4c3dcac
// Register custom serializer for E2E testing of persistent component state serialization extensibility | ||
builder.Services.AddSingleton<IPersistentComponentStateSerializer<int>, CustomIntSerializer>(); |
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.
Do we need this? Since it's already on WasmMinimal
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.
You're correct - the registration is not needed since E2E tests use Components.WasmMinimal for WebAssembly rendering, not BasicTestApp. Removed the redundant registration and unused using directive. Commit 3b3af1d
…ateSerializer<T> and make base interface public Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// Provides custom serialization logic for persistent component state values of type <typeparamref name="T"/>. | ||
/// </summary> | ||
/// <typeparam name="T">The type of the value to serialize.</typeparam> | ||
public interface IPersistentComponentStateSerializer<T> : IPersistentComponentStateSerializer |
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.
Make this a class PersistentComponentStateSerializer<T>
and make the interface internal
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.
Converted IPersistentComponentStateSerializer<T> interface to abstract class PersistentComponentStateSerializer<T> and made the base interface internal as requested. Updated all implementations, tests, and service registrations accordingly. Commit 970977b
…tract class PersistentComponentStateSerializer<T> Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// <param name="value">The value to serialize.</param> | ||
/// <param name="writer">The buffer writer to write the serialized data to.</param> | ||
/// <returns>A task that represents the asynchronous serialization operation.</returns> | ||
public abstract Task PersistAsync(T value, IBufferWriter<byte> writer); |
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.
Make this method void returning and drop the Async suffix
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.
Made the PersistAsync method void returning as requested. Updated the interface, abstract class, all implementations, and test methods. All 45 persistent state tests continue to pass. Commit 0d5d8df
|
||
internal interface IPersistentComponentStateSerializer | ||
{ | ||
Task PersistAsync(Type type, object value, IBufferWriter<byte> writer); |
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.
Drop the task and the async suffix (return void)
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.
The interface method is already void returning as requested. This was addressed in a previous commit (0d5d8df).
private IPersistentComponentStateSerializer? ResolveSerializer(Type type) | ||
{ | ||
return _serializerCache.GetOrAdd(type, SerializerFactory); | ||
} |
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.
Inline this method call
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.
Inlined the ResolveSerializer method call at both usage sites and removed the method definition. Commit e922076
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
/// </summary> | ||
/// <param name="value">The value to serialize.</param> | ||
/// <param name="writer">The buffer writer to write the serialized data to.</param> | ||
public abstract void PersistAsync(T value, IBufferWriter<byte> writer); |
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.
Drop the async suffix from the method name here and on the base interface
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.
Dropped the "Async" suffix from both the interface and abstract class method names since they now return void instead of Task. Updated all implementations and tests accordingly. Commit ebe6f52
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
Co-authored-by: javiercn <6995051+javiercn@users.noreply.github.com>
This PR implements serialization extensibility for declarative persistent component state, allowing developers to provide custom serializers beyond the built-in JSON serialization.
Changes
New Interface
Key Features
PersistAsync
is async for serialization work, whileRestore
is synchronous to prevent UI tearingIBufferWriter<byte>
withPooledArrayBufferWriter
to minimize allocations, avoidingbyte[]
APIsUsage Example
Register custom serializers in DI:
Components work exactly as before:
Implementation Details
PersistAsync<T>
andTryTake<T>
methods for custom serializersTesting
This enables scenarios like compression, encryption, or optimized binary formats for specific types while maintaining the simple declarative syntax of
[PersistentState]
.Fixes #60716.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.