SignalR SourceGenerator attribute API

BrennanConroy commented 2 years ago

Background and Motivation

For PR https://github.com/dotnet/aspnetcore/pull/38025

We are adding a source generator for SignalR .NET clients to allow sharing strongly-typed interfaces between client and server. We have had the server side since first release and are now adding support for the client-side.

Proposed API

namespace Microsoft.AspNetCore.SignalR.Client
{
+    [AttributeUsage(AttributeTargets.Method)]
+    public sealed class ClientHubAttribute : Attribute
+    {
+    }

+    [AttributeUsage(AttributeTargets.Method)]
+    public sealed class ServerHubProxyAttribute : Attribute
+    {
+    }
}

Package Name: Microsoft.AspNetCore.SignalR.Client.SourceGenerator Attribute DLL Name: Microsoft.AspNetCore.SignalR.Client.SourceGenerator.HubProxyAttributes.dll

Usage Examples

internal static partial class ProxyExtensions
{
    [ServerHubProxy]
    public static partial T ServerHub<T>(this HubConnection conn);

    [ClientHub]
    public static partial IDisposable CallbackRegistration<T>(this HubConnection conn, T impl);
}

public interface IMyHub
{
    Task GetNothing();
}

public interface IMyHubClient
{
    Task ReceiveMessage(string message);
}

public class MyHubClient : IMyHubClient
{
    public Task ReceiveMessage(string message)
    {
        Console.WriteLine(message);
        return Task.CompletedTask;
    }
}

var myHub = conn.ServerHub<IMyHub>();
var dispose = connection.CallbackRegistration<IMyHubClient>(new MyHubClient());
await myHub.GetNothing();

Alternative Designs

HubServerProxyAttribute

HubClientProxyAttribute ClientMethodsAttribute SignalRClientAttribute ClientRegistrationAttribute

ghost commented 2 years ago

Thank you for submitting this for API review. This will be reviewed by @dotnet/aspnet-api-review at the next meeting of the ASP.NET Core API Review group. Please ensure you take a look at the API review process documentation and ensure that:

The PR contains changes to the reference-assembly that describe the API change. Or, you have included a snippet of reference-assembly-style code that illustrates the API change.
The PR describes the impact to users, both positive (useful new APIs) and negative (breaking changes).
Someone is assigned to "champion" this change in the meeting, and they understand the impact and design of the change.

ghost commented 2 years ago

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

pranavkm commented 2 years ago

API review: We were still figuring out the names. The runtime recently added a source generator for P/Invokes and used the name: GeneratedDllImportAttribute. It would be nice to try and use that as a naming pattern for similar generated code across frameworks. However we could not come up with a similar name that wasn't also too verbose. For now, the two best options we could come up with were:

{
+    [AttributeUsage(AttributeTargets.Method)]
+    public sealed class ClientRegistrationAttribute : Attribute
+    {
+    }

+    [AttributeUsage(AttributeTargets.Method)]
+    public sealed class ServerHubProxyAttribute : Attribute
+    {
+    }

@BrennanConroy will talk to the remaining SignalR team and determine if these names are good to proceed with.

sommmen commented 8 months ago

@BrennanConroy

Hello, it seems a preview package is on nuget already - i was wondering about the status of client generation for signalR, would the preview package be production ready and will this feature come to net8?

BrennanConroy commented 8 months ago

would the preview package be production ready and will this feature come to net8?

No. See current thoughts on what needs to be done still https://github.com/dotnet/aspnetcore/issues/32534#issuecomment-1668184279

dotnet / aspnetcore