-
Notifications
You must be signed in to change notification settings - Fork 164
Add RpcTargetMetadata.FromShape<T> overloads that work on .NET Framework
#1358
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
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 .NET Framework-compatible overloads of RpcTargetMetadata.FromShape<T> methods using C# 14 extension static methods. The PR enables multi-targeting code to use the same convenient syntax across both .NET and .NET Framework, with the compiler automatically selecting the appropriate implementation based on the target framework.
- Adds extension static methods in a new
RpcTargetMetadataExtensionsclass to provide .NET Framework-compatible overloads - Updates target frameworks to include net9.0 for both the library and tests
- Adds
RequiresUnreferencedCodeattributes to classes using reflection inJsonMessageFormatter.cs
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/StreamJsonRpc/RpcTargetMetadataExtensions.cs | New file containing extension static methods for .NET Framework-compatible FromShape overloads |
| src/StreamJsonRpc/StreamJsonRpc.csproj | Adds net9.0 target framework and suppresses StyleCop SA1201 for new C# 14 extension syntax |
| src/StreamJsonRpc/JsonMessageFormatter.cs | Adds RequiresUnreferencedCode attributes to async enumerable converter classes |
| test/StreamJsonRpc.Tests/StreamJsonRpc.Tests.csproj | Adds net9.0 target framework |
| test/StreamJsonRpc.Tests/RpcTargetMetadataTests.cs | Updates test to use parameterless FromShape<T>() method, removing explicit provider argument |
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.
![]()
c72f8cd to
52de5da
Compare
|
docfx is broken by this change. We'll have to block this change on dotnet/docfx#10808 |
f2032c2 to
ef663d4
Compare
55d7b61 to
80109a4
Compare
This involves some fancy syntax. Here's an explanation:
The preferred APIs are .NET-only. These are the generic methods with a type constraint of
IShapeable<T>. But this interface has a static member, making it .NET-only.But
FromShape<T>is a convenient method to call, and fast too. We'd like to expose an overload of the method that doesn't have the generic type constraint, since we can implement it properly on .NET Framework. But C# does not let us declare two overloads that vary only by generic type constraint on the same type.But we can declare such overloads when one is on an extension class instead. For instance methods, we could use ordinary extension method syntax. But since the methods to be overloaded are themselves
static, we must use the new C# 14 extension members syntax, which permits extension static methods.This allows multi-targeting code (such as our tests which I made a 1-line change to in order to exercise the new method) to simply use the preferred syntax and the compiler will automatically choose the best method based on the target framework.
The
#if'd attributes on the new methods are to guide users when the compiler doesn't choose the preferred overload on .NET, since when that happens, it is an indication that the code will fail at runtime.The
RequiresDynamicCodeattribute is a subtle requirement of PolyType only on .NET 8. .NET 9+ no longer has that requirement.