-
Notifications
You must be signed in to change notification settings - Fork 164
Add PolyTypeJsonFormatter as an experimental API
#1344
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
Conversation
f58d409 to
e31485b
Compare
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 adds a new PolyTypeJsonFormatter class that provides NativeAOT-compatible JSON serialization for StreamJsonRpc using PolyType. The formatter is marked as experimental and enables AOT scenarios where dynamic code generation isn't available.
Key changes:
- Introduces
PolyTypeJsonFormatteras a new formatter option with PolyType-based serialization - Refactors
RequestIdJsonConverterinto a sharedRequestIdSTJsonConverterfor use across formatters - Creates supporting infrastructure classes (
IGenericTypeArgAssist,SourceGenerationContext) - Adds comprehensive test coverage with multiple test classes for different scenarios
Reviewed Changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 26 comments.
Show a summary per file
| File | Description |
|---|---|
| src/StreamJsonRpc/PolyTypeJsonFormatter.cs | New formatter implementation with PolyType support and experimental attribute |
| src/StreamJsonRpc/RequestIdSTJsonConverter.cs | Extracted RequestId converter for System.Text.Json reuse |
| src/StreamJsonRpc/SourceGenerationContext.cs | Source generation context for built-in types |
| src/StreamJsonRpc/IGenericTypeArgAssist.cs | Helper interfaces for generic type argument handling |
| src/StreamJsonRpc/RequestId.cs | Added System.Text.Json converter attribute |
| src/StreamJsonRpc/SystemTextJsonFormatter.cs | Refactored to use shared RequestIdSTJsonConverter |
| src/StreamJsonRpc/StreamJsonRpc.csproj | Whitespace formatting fixes |
| test files | Test classes for PolyTypeJsonFormatter covering various scenarios |
Comments suppressed due to low confidence (2)
src/StreamJsonRpc/PolyTypeJsonFormatter.cs:1347
- This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
src/StreamJsonRpc/PolyTypeJsonFormatter.cs:1359 - This foreach loop implicitly filters its target sequence - consider filtering the sequence explicitly using '.Where(...)'.
etvorun
left a comment
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.
![]()
e31485b to
9254613
Compare
This is a formatter intended for use in the vs-servicehub repo to support FrameworkServices so that we can have a full stack brokered service system that is NativeAOT safe.
The formatter is hard to use, and will likely evolve into a fully PolyType-based JSON serializer. In the meantime, I have
[Experimental]slapped onto it to avoid having to maintain the public API for any user outside of vs-servicehub.