dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.55k stars 10.05k forks source link

[Design proposal] Strongly-typed SignalR hub proxies and callback handlers #32534

Open mehmetakbulut opened 3 years ago

mehmetakbulut commented 3 years ago

Summary

This proposal aims to provide end-to-end strong typing for all interactions between SignalR clients and SignalR hubs.

Motivation and goals

In scope

  1. Strongly-typed remote procedure calls from a SignalR client to a SignalR hub
  2. Strongly-typed streaming calls from a SignalR client to a SignalR hub
    1. Support for both single direction streaming in either direction and bi-directional streaming
    2. Support for cancellation token in streaming calls.
    3. Support for both ChannelReader<> and IAsyncEnumerable<> as the underlying stream
  3. Strongly-typed handlers for calls from a SignalR hub to a SignalR client

Out of scope

  1. Non-C# usage. For the time being, we should focus on design and implementation of the ideal solution. Later we can think about how this design can be implemented in languages other than C# based on collaboration with the community.

Risks / unknowns

  1. Transformative features (e.g. HubMethodName) can break strongly-typed clients depending on the particular implementation.
    1. Should we allow interface definitions to be annotated with attributes so the consumers can account for a subset of these issues?
  2. Different implementations have different performance.
    1. Is there a level of performance that we would want to guarantee across all target platforms?
  3. Some designs may not be possible to implement for some platforms.
    1. Which platforms should we target?
    2. Would we be OK with different implementations for different platforms?
  4. Versioning may be hard.

Examples

Client to Server Calls

Let's say there is an interface IMyHub as defined below which is implemented by an ASP.NET Core application as MyHub : SignalR.Hub, IMyHub.

    public interface IMyHub
    {
        Task Do();

        Task<int> Get();

        Task Set(int a);

        Task<int> GetAndSet(int a);

        Task<ChannelReader<int>> StreamToClientViaChannel();

        Task<ChannelReader<int>> StreamToClientViaChannelWithToken(CancellationToken cancellationToken);

        Task StreamFromClientViaChannel(ChannelReader<int> reader);

        IAsyncEnumerable<int> StreamToClientViaEnumerableWithToken([EnumeratorCancellation] CancellationToken cancellationToken);

        Task StreamFromClientViaEnumerable(IAsyncEnumerable<int> reader);
    }

A developer currently needs to consume it as below.

await hubConnection.InvokeAsync("Do");
var ret = (int) await hubConnection.SendAsync("GetAndSet", 100);

Instead, developer could be making strongly-typed calls.

await myHub.Do();
var ret = await myHub.GetAndSet(100);

We can either have such a proxy be acquired from a hub connection or a builder.

var myHub = myHubConnection.AsHubProxy<IMyHub>();
// vs
var myHub = new HubProxyBuilder<IMyHub>()
    .WithConnection(myHubConnection)
    .ConfigureAnythingElse()
    .Build();

Acquisiton from a hub connection is simpler while builder provides more room for extension in future.

Server to Client Call Handlers

One can similarly define an interface IMyClient as below which can then be used in Hub<IMyClient> on server-end and implemented by any consumer to provide callbacks.

    public interface IMyClient
    {
        void Callback1();
        void Callback2(string arg);
    }

A developer currently needs to provide such callbacks as below.

await hubConnection.On("Callback1", (req) => { someActivity1(); });
await hubConnection.On<string>("Callback2", (arg) => { someActivity2(arg); });

Instead, developer could be registering callback in a strongly-typed manner.

public class MyClient : IMyClient { .. }
var myClient = new MyClient();
await hubConnection.RegisterCallbacks<IMyClient>(myClient);

Multiple callback providers can be registered against the same hub connection so different callbacks can be provided by different classes. However this does mean overlap is possible. We'd want to decide how to handle this and whether to impose restrictions.

Detailed design

To be decided.

Some alternatives for client-to-server call implementation are: Source-generated proxies utilizes C# 9 / .NET 5 source generator feature.

Pros:

Cons:

Dynamic proxies utilizes Reflection.Emit to dynamically generate proxy.

Pros:

Cons:

Expressive proxies utilizes expressions to emulate a strongly-typed user experience.

Pros:

Cons:

Server-to-client calls can be registered with just reflection (no Reflection.Emit) which is simple enough and would work on practically any platform. Other alternatives are possible such as source generation as well.

ghost commented 3 years ago

Thanks for contacting us.

We're moving this issue to the Next sprint 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.

BrennanConroy commented 3 years ago

await hubConnection.RegisterCallbacks(myClient); Multiple callback providers can be registered against the same hub connection so different callbacks can be provided by different classes. However this does mean overlap is possible. We'd want to decide how to handle this and whether to impose restrictions.

My main issue with this approach is that you are forced to define all methods on the client side. We've seen many cases where some clients don't register for all methods. And you can't just have them throw NotImplementedException because that isn't a good design and the SignalR library will just swallow that (we probably log as well).

client-to-server call implementation are:

Source-generated proxies is my favorite. It should work everywhere (in some environments you might need to generate the code before using it but that shouldn't be too hard).

Transformative features (e.g. HubMethodName) can break strongly-typed clients depending on the particular implementation.

1. Should we allow interface definitions to be annotated with attributes so the consumers can account for a subset of these issues?

I'd say this is a P2 concern. We don't support this today, but if we add support for it, then we'll update the proxy to handle it as well.

Different implementations have different performance.

1. Is there a level of performance that we would want to guarantee across all target platforms?

As fast as possible 😄 Ideally there shouldn't be very much observable perf difference with this. Most of the code will be a pass-through wrapper right?

Some designs may not be possible to implement for some platforms.

1. Which platforms should we target?

2. Would we be OK with different implementations for different platforms?

I don't think a different implementation for different platforms is good. We already feel the burden for that sort of thing by having different clients for different languages. And it looks like we'll already be needing a different proxy generation for .NET vs. Typescript.

We should be targeting everything .NET Core runs on. i.e Xamarin, WPF, Console, WASM, etc.

4\. Versioning may be hard.

What versioning are you envisioning? So far we haven't made any breaking changes to the protocol or main APIs and aren't planning on doing that. New APIs will not be used by old proxy generators which should be fine. Although if possible we can try to make new proxy generation work on older clients.

var myHub = myHubConnection.AsHubProxy<IMyHub>();
// vs
var myHub = new HubProxyBuilder<IMyHub>()
    .WithConnection(myHubConnection)
    .ConfigureAnythingElse()
    .Build();

I currently like the first option as it allows use of both the strongly typed connection, as well as the less-typed connection. Although I guess the type returned from the builder could include a property with the less-typed connection.

mehmetakbulut commented 3 years ago

My main issue with this approach is that you are forced to define all methods on the client side.

That makes sense. I'm OK with restricting registration to a single "callback provider". However are you saying you don't like await hubConnection.RegisterCallbacks<IMyClient>(myClient); because IMyClient could be defined by client instead of server?

I was envisioning that IMyClient would normally be defined by server and used as Hub<IMyClient> as well as provided to clients as part of a library or common project.

Source-generated proxies is my favorite.

Agreed. I plan to focus on a detailed design based on source generation unless there is strong opposition.

I'd say this is a P2 concern.

:thumbsup:

Most of the code will be a pass-through wrapper right?

With source generation, it is just one extra vcall. So yes :)

And it looks like we'll already be needing a different proxy generation for .NET vs. Typescript.

Yes. Are you envisioning .NET source-generators would generate Typescript source or some other more Typescript-native feature would be utilized? I don't use Typescript so I'd definitely want more feedback on how we could approach that.

What versioning are you envisioning?

Versioning of the interfaces defined by users. When I make changes to IMyHub, if the interface is used by third parties, I'm really only limited to backwards compatible changes. I suspect there isn't much that can be done here. It could always be a second/third pass addition..

I currently like the first option as it allows use of both the strongly typed connection, as well as the less-typed connection.

Same. I think for a first pass also it reduces complexity.

BrennanConroy commented 3 years ago

However are you saying you don't like await hubConnection.RegisterCallbacks<IMyClient>(myClient); because IMyClient could be defined by client instead of server?

No, I'm saying that myClient is a class that must define all methods. It is a nice way of doing things though, you just implement a class that inherits IMyClient, so we might want to consider keeping it and also providing a way to register individual methods?

Are you envisioning .NET source-generators would generate Typescript source or some other more Typescript-native feature would be utilized? I don't use Typescript so I'd definitely want more feedback on how we could approach that.

I'm not sure yet. I believe there are .NET libraries that can produce JS or TS, so we might want to try and structure the source-generator in a way that we can possibly plug in other language generators in the future.

Versioning of the interfaces defined by users. When I make changes to IMyHub, if the interface is used by third parties, I'm really only limited to backwards compatible changes. I suspect there isn't much that can be done here. It could always be a second/third pass addition..

Yeah, not sure what we can do there :)

BrennanConroy commented 3 years ago

Would you like to propose what the API will look like and we can start iterating on it?

nenoNaninu commented 3 years ago

Another proposal

I have published a library called "TypedSignalR.Client" which is Source Generator to create a strongly typed SignalR Client. So this proposal has an implementation(slightly different from here).

I think my proposal can address some of the current requirements.

Summary

The work flow for developers is as follows.

  1. Annotate Attribute to generate code.
  2. Inherit the generated class and override only the necessary methods to implement.
  3. Call Hub through the implemented class.

Detail and Example

First, suppose we have the following interface or class definition. This interface and class definition are assumed to be shared in the server and client projects by project reference etc.

public class UserDefine
{
    public Guid RandomId { get; set; }
    public DateTime Datetime { get; set; }
}

// The return type of the client-side method must be Task. 
// This is because Hub<T> will cause a runtime error if a non-Task type is returned.
public interface IClientContract
{
    // Of course, user defined type is OK. 
    Task SomeClientMethod1(string user, string message, UserDefine userDefine);
    Task SomeClientMethod2();
}

// The return type of the method on the hub-side must be Task or Task <T>. 
public interface IHubContract
{
    Task<string> SomeHubMethod1(string user, string message);
    Task SomeHubMethod2();
}

And suppose that it is implemented on the server as follows.

using Microsoft.AspNetCore.SignalR;

public class SomeHub : Hub<IClientContract>, IHubContract
{
    public async Task<string> SomeHubMethod1(string user, string message)
    {
        await this.Clients.All.SomeClientMethod1(user, message, new UserDefineClass());
        return "OK!";
    }

    public async Task SomeHubMethod2()
    {
        await this.Clients.Caller.SomeClientMethod2();
    }
}

Under these assumptions, on the client side, annotate the HubClientBaseAttribute to the partial class as follows.

using TypedSignalR.Client;

[HubClientBase(typeof(IHubContract), typeof(IClientContract))]
partial class ClientBase
{
}

By annotating the HubClientBaseAttribute, the following code will be generated (simplified here).

partial abstract class ClientBase : IHubClient<IHubContract>, IClientContract, IAsyncDisposable
{
    private class HubInvoker : IHubContract
    {
        private readonly HubConnection _connection;

        public HubInvoker(HubConnection connection)
        {
            _connection = connection;
        }

        public Task<string> SomeHubMethod1(string user,string message)
        {
            return _connection.InvokeAsync<string>(nameof(SomeHubMethod1), user, message);
        }

        public Task SomeHubMethod2()
        {
            return _connection.InvokeAsync(nameof(SomeHubMethod2));
        }
    } // class HubInvoker

    public HubConnection Connection { get; }
    public IHubContract Hub { get; }
    protected List<IDisposable> disposableList = new();

    public ClientBase(HubConnection connection)
    {
        Connection = connection;
        Hub = new HubInvoker(connection);

        Connection.Closed += OnClosed;
        Connection.Reconnected += OnReconnected;
        Connection.Reconnecting += OnReconnecting;

        var d1 = Connection.On<string, string, UserDefineClass>(nameof(SomeClientMethod1), SomeClientMethod1);
        var d2 = Connection.On(nameof(SomeClientMethod2), SomeClientMethod2);

        disposableList.Add(d1);
        disposableList.Add(d2);
    }

    public virtual Task SomeClientMethod1(string user,string message, UserDefineClass userDefine) => Task.CompletedTask;

    public virtual Task SomeClientMethod2() => Task.CompletedTask;

    public async ValueTask DisposeAsync()
    {
        Connection.Closed -= OnClosed;
        Connection.Reconnected -= OnReconnected;
        Connection.Reconnecting -= OnReconnecting;

        await Connection.DisposeAsync();

        foreach(var d in disposableList)
        {
            d.Dispose();
        }
    }

    public virtual Task OnClosed(Exception e) => Task.CompletedTask;
    public virtual Task OnReconnected(string connectionId) => Task.CompletedTask;
    public virtual Task OnReconnecting(Exception e) => Task.CompletedTask;
} // class ClientBase

The generated code is inherited and used. The usability is very similar to the case of inheriting Hub<T>.

Also, the definition of the function is not a callback, but writing feeling similar to Hub-side.

class HubClient : ClientBase
{
    public HubClient(HubConnection connection, string arg) : base(connection)
    {
    }

    // override and impl
    public override async Task SomeClientMethod1(string user, string message, UserDefineClass userDefine)
    {
        await this.Hub.SomeHubMethod1(string user, string message);
        Console.WriteLine("Call SomeClientMethod1!");
        return Task.CompletedTask;
    }

    // We don't have to implement all the methods !
    // public override Task SomeClientMethod2()

    // I think it is very comfortable to write SignalR event as follows.
    public override Task OnClosed(Exception e)
    {
        Console.WriteLine($"[On Closed!]");
        return Task.CompletedTask;
    }

    // Of course, we don't have to define all events.
    // public override Task OnReconnecting(Exception e)
}

Since the base class comes with an implementation, developers can selectively override it, not all functions need to be implemented.

It's also easy to use.

HubConnection connection = ...;

var client = new HubClient(connection, "some parameter");

await client.Connection.StartAsync();

// Invoke hub methods
var response = await client.Hub.SomeHubMethod1("user", "message");
Console.WriteLine(response);

await client.Connection.StopAsync();
await client.DisposeAsync();

Personally, I don't hete writing new HubClient(connection, "some parameter"). If you are not comfortable with this, how about the following API?

public static T Build<T>(this IHubConnectionBuilder source, Func<HubConnection,T> factoryMethod)
{
    HubConnection connection = source.Build();
    return factory.Invoke(connection);
}

static void Main(string[] args)
{
    var client = new HubConnectionBuilder()
        .WithUrl("https://~~~")
        .Build(connection => new HubClient(connection, "some parameter"));
}
mehmetakbulut commented 3 years ago

I'd suggest the following basic API surface.

// Get strongly typed hub proxy
public T GetProxy<T>(this HubConnection conn);

// Register callback method and get a disposable which can unregister callback
public IDisposable RegisterCallback<T1, T2, ..>(this HubConnection conn, string name, Action<T1, T2, ..> callback);

// Register callback provider (i.e. a class providing the callback methods defined in TInterface) and get a disposable which can unregister callbacks
public IDisposable RegisterCallbackProvider<TInterface, TImplementation>(this HubConnection conn, TImplementation callbackProvider) where TImplementation : TInterface;

This keeps it simple to grab a proxy as well as register single and/or multiple callbacks. I'm not very certain of the single callback registration because it is not a use case I had before. I'm a bit worried that it may be not much better than what SignalR already offers.

mehmetakbulut commented 3 years ago

@nenoNaninu It appears what you are proposing is one layer higher than what I've proposed here. Your proposal essentially wraps proxy acquisition and callback registration behind a single base client class which seem to serve almost as a replacement for HubConnection (e.g. you also provide automatic subscriptions for connection/disconnection etc..).

My intent is a bit different. I'd like to see the essential strong typing features implemented without imposing conditions on implementations within end-user codebase (i.e. not require inheritance of an abstract class) and then such abstractions as yours could be added on top very easily.

BrennanConroy commented 3 years ago

public IDisposable RegisterCallback<T1, T2, ..>(this HubConnection conn, string name, Action<T1, T2, ..> callback);

Now that I think about it, this is just hubConnection.On<T>(string name, ...). For some reason I was thinking there would be a difference. We probably don't want that. It would be interesting if there was a more strongly-typed API like, hubProxy.MethodName.Register((param) => Console.WriteLine(param)); food for thought. But I do think we should not have RegisterCallback in it's current state since it adds nothing over .On.

public IDisposable RegisterCallbackProvider<TInterface, TImplementation>(this HubConnection conn, TImplementation callbackProvider) where TImplementation : TInterface;

Should this be taking the HubConnection or the proxy type? And if it's on the proxy type, it wouldn't need to be an extension method, and the TInterface could be pre-defined based on the GetProxy call earlier.

public T GetProxy(this HubConnection conn);

This should return the proxy type right? We also want to see what the proxy type looks like.

mehmetakbulut commented 3 years ago

Here is a more concrete proxy example. Given following interface in a shared/common project/library:

public interface IMyHub
{
    Task DoSomething();
    Task<int> DoSomethingElse(float arg);
    IAsyncEnumerable<int> StreamBidirectional(IAsyncEnumerable<float> clientToServer, [EnumeratorCancellation] CancellationToken token);
}

Generated proxy would be: (types would be written in fully qualified form but just simplifying for showing..)

public sealed class GeneratedIMyHub : IMyHub
{
    private readonly HubConnection conn;
    public GeneratedIMyHub(HubConnection connection)
    {
        this.conn = connection;
    }

    public Task DoSomething()
    {
        return this.conn.InvokeAsync("DoSomething");
    }

    public Task<int> DoSomethingElse(float arg)
    {
        return this.conn.InvokeAsync<int>("DoSomethingElse", arg);
    }

    public IAsyncEnumerable<int> StreamBidirectional(IAsyncEnumerable<float> clientToServer, CancellationToken token)
    {
        return this.conn.StreamAsync<int>("StreamBidrectional", clientToServer, token);
    }
}

Methods with void rtype are the only caveat. These would be called via SendAsync but that returns a task so we have to ignore or discard the returned task.

GeneratedIMyHub is an implementation detail that is not shown to the user on any API (though they can browse to source if they want). When the user calls hubConnection.GetProxy<IMyHub>(), they just get an object that satisfies IMyHub which happens to really be of type GeneratedIMyHub.

Should this be taking the HubConnection or the proxy type?

RegisterCallbackProvider should indeed take the HubConnection object and not the proxy type or the proxy object. This is because the callbacks aren't related to the strongly-typed representation of the hub. TInterface here would nominally be a IMyClient published/shared by server in a common project/library. In my opinion, we shouldn't tie this to IMyHub which is implemented by proxy as well as the actual hub.

In the case of RegisterCallbackProvider, you can imagine two variants:

public IDisposable RegisterCallbackProvider<TInterface, TImplementation>(this HubConnection conn, TImplementation callbackProvider) where TImplementation : TInterface;

public IDisposable RegisterCallbackProvider<TClient>(this HubConnection conn, TClient callbackProvider);

Underlying code would look like:

public IDisposable RegisterCallbackProvider<TInterface, TImplementation>(this HubConnection conn, TImplementation callbackProvider) where TImplementation : TInterface
{
    var intfType = typeof(TInterface);
    var regs = new List<IDisposable>();
    var methods = intfType.GetMethods(BindingFlags.Instance | BindingFlags.Public);
    foreach (var method in methods)
    {
        var reg = conn.On(method.Name, method.GetParameters().Select(a => a.ParameterType).ToArray(),
                objects =>
                    {
                        method.Invoke(spoke, objects);
                        return Task.CompletedTask;
                    });
                regs.Add(reg);
    }
    return MakeDisposableOf(regs);
}

public IDisposable RegisterCallbackProvider<TClient>(this HubConnection conn, TClient callbackProvider)
{
    return RegisterCallbackProvider<TClient, TClient>(conn, callbackProvider);
}

As a more concrete example, we can have the following interface in our shared/common project:

public interface IMyClient
{
    void SomeCallback(float arg);
}

In client project, user can utilize a callback provider like:

public class MyClient : IMyClient
{
    public void SomeCallback(float arg) { .. }
}

var registration = hubConnection.RegisterCallbackProvider<IMyClient, MyClient>(new MyClient());
// or hubConnection.RegisterCallbackProvider(new MyClient());
registration.Dispose();

There aren't many restrictions on what could be a callback provider. For example, if the user doesn't have a IMyClient, they can still register a provider by using the 2nd overload. That would just use all public instance methods as the "interface" so the user would need to ensure they don't have public methods that aren't meant to be callable. Though we can restrict registration to always supply an interface (and remove the 2nd overload) if we don't like this option.

BrennanConroy commented 3 years ago

Couple notes:

IDisposable RegisterCallbackProvider<TInterface>(TInterface impl)
{
    var disposableCollection;
    // for loop is just for pseudo code purposes, this would be source generated to have individual calls for each .On callback
    foreach (method)
    {
        disposableCollection.Add(conn.On("blah", (param1) => impl.blah(param1)));
    }

    return disposableCollection;
}
bradygaster commented 3 years ago

@mehmetakbulut - would you mind reaching out with your email or a good way to schedule you for a teams meeting? The team likes your approach and discussed some ideas we have around it. We'd like to iterate on our design ideas and the discussion we had, so if you could provide a way for us to get in touch we'd like to schedule a meeting with you. You could also DM me on twitter at @bradygaster if that is easier (ping me so I can follow so we can DM if that works).

mehmetakbulut commented 3 years ago

@bradygaster I just sent you an email from my personal.

BrennanConroy commented 3 years ago
HubConnection connection;
connection.GetProxy<T>(); // find "GetProxy", stash syntax node for second pass
// example:
// https://github.com/davidfowl/uController/blob/aa8bcb4b30764e42bd72d326cb2718a4d4eaf4a9/src/uController.SourceGenerator/uControllerGenerator.cs#L163-L179
// https://github.com/davidfowl/uController/blob/aa8bcb4b30764e42bd72d326cb2718a4d4eaf4a9/src/uController.SourceGenerator/uControllerGenerator.cs#L37-L43

Try a similar method for RegisterCallbackProvider as well.

For testing: We can consider adding IHubConnection to make unit testing possible/easier. Today you need to either do E2E testing or something super hacky to verify that the correct HubConnection methods are called.

BrennanConroy commented 3 years ago

Initial source generator work has been merged! Thanks a ton @mehmetakbulut!

To use the source generator:

  1. add a reference to Microsoft.AspNetCore.SignalR.Client.SourceGenerator from feed https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet7%40Local/nuget/v3/index.json
  2. add a HubServerProxyAttribute class to your project:
    [AttributeUsage(AttributeTargets.Method)]
    internal class HubServerProxyAttribute : Attribute
    {
    }
  3. add a HubClientProxyAttribute class to your project:
    [AttributeUsage(AttributeTargets.Method)]
    internal class HubClientProxyAttribute : Attribute
    {
    }
  4. add a static partial class to your project and write a static partial method with the [HubClientProxy] attribute
    internal static partial class MyCustomExtensions
    {
    [HubClientProxy]
    public static partial IDisposable AnyNameYouWant<T>(this HubConnection connection, T provider);
    }
  5. add a static partial class to your project (could be the same as step 4) and write a static partial method with the [HubServerProxy] attribute
    internal static partial class MyCustomExtensions
    {
    [HubServerProxy]
    public static partial T AnotherName<T>(this HubConnection connection);
    }
  6. use the partial methods from your code!
    
    public interface IServerHub
    {
    Task SendMessage(string message);
    Task<int> Echo(int i);
    }
    public interface IClient
    {
    Task ReceiveMessage(string message);
    }
    public class Client : IClient
    {
    // Equivalent to HubConnection.On("ReceiveMessage", (message) => {});
    Task ReceiveMessage(string message)
    {
        return Task.CompletedTask;
    }
    }

HubConnection connection = new HubConnectionBuilder().WithUrl("...").Build(); var stronglyTypedConnection = connection.AnotherName(); var registrations = connection.AnyNameYouWant(new Client());

await stronglyTypedConnection.SendMessage("Hello world"); var echo = await stronglyTypedConnection.Echo(10);



Follow up source generator work:
- [ ] Add `HubClientProxyAttribute` and `HubServerProxyAttribute` to the product
  - Put it in the client dll or add a dll to the source generator with these classes
- [ ] remove `<T>` requirement from methods
- [ ] allow non-extension method syntax `private static partial IServerHub GetProxy(HubConnection connection);`
- [ ] support multiple attributed methods (currently limited to 1 HubClientProxy and 1 HubServerProxy)
  - [ ] Test with a parameter name other than `provider`. https://github.com/dotnet/aspnetcore/pull/38025/files#r741500461
- [ ] Test diagnostic messages, like https://github.com/dotnet/aspnetcore/blob/cf5d3ce8c9f226cc436aedb1cfa910146b36b90d/src/Analyzers/Analyzers/test/StartupAnalyzerTestBase.cs#L66
- [ ] See how the experience feels and improve/add more diagnostics
- [ ] code-fix for writing the methods? Triggers on writing the attribute maybe? `[HubServerProxy]`
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.

markushaslinger commented 1 year ago

There are several similar issues regarding this topic, so I hope this is the proper place to ask: Microsoft.AspNetCore.SignalR.Client.SourceGenerator was previewed during the .NET 7 dev phase, but, according to the package version up on NuGet, never left the preview phase. Now we are nearing the end of the .NET 8 dev phase and I'd like to know what the plan for this package is - @BrennanConroy are you still working on this?

If there aren't any issues, it could be made a full release, thus projects which do not allow using preview packages could pick it up. Completing the tasks listed above and adding it to the signalr library as originally planned (if I read that correctly) could still happen for .NET 9, while allowing the already working pieces to be used earlier.

BrennanConroy commented 1 year ago

If there aren't any issues, it could be made a full release

We aren't sure about the current API shape so won't be making a full release until then.

We had some thoughts on how the API could be changed, basically adding some new methods to HubConnectionBuilder and having them fallback to reflection if someone doesn't use the source-generator. The source-generator would be preferred over the reflection of course, but this would allow usage without a source-generator.

Rough API shape

// scenario1: strongly typed to server, but manual .On needed for receiving
HubConnection<IServerHub> hubConnection = new HubConnectionBuilder()
    .WithUrl(http://localhost:5000/default)
   .Build<IServerHub>();

// scenario2: Manual sends to server, strongly-typed from server 
HubConnection hubConnection = new HubConnectionBuilder()
    .WithUrl(http://localhost:5000/default)
    .WithClientHub<ChatClient>()
    //.WithClientHub(new ChatClient())
    .Build();

// scenario3: strongly-typed to and from server
HubConnection<IServerHub> hubConnection = new HubConnectionBuilder()
    .WithUrl(http://localhost:5000/default)
    .WithClientHub<ChatClient>()
    //.WithClientHub(new ChatClient())
    .Build<IServerHub>();

public abstract class ClientHub<T>
{
    public virtual Task OnConnectedAsync() => Task.CompletedTask;

    public virtual Task OnReconnectingAsync(RetryContext retry) => Task.CompletedTask;
    public virtual Task OnReconnectedAsync() => Task.CompletedTask;

    public virtual Task OnDisconnectedAsync(Exception? ex) => Task.CompletedTask;

    public T Server { get; set; }

    // Might want HubConnection<T>
    public HubConnection HubConnection { get; set; }
}

// User code for bi-directional client-side hub
public class ChatClient : ClientHub<IServerHub>, IMyHubClient
{
    public ChatClient(IService service)
    {
    }

    public override Task OnConnectedAsync()
    {
        return Task.CompletedTask;
    }

    public override Task OnDisconnectedAsync(Exception? ex)
    {
        return Task.CompletedTask;
    }

    public Task ReceiveMessage(string message)
    {
        return Server.Send("Proxy", message);
    }
}
markushaslinger commented 1 year ago

I know you have many more scenarios on your plate, but from my point of view I’d say: don’t overthink it.

Applications I’m building today (and I’m talking about current .NET with awesome cross-plat capabilities here) are .NET backend & .NET frontend. The backend will almost always be ASP.NET Core, and the frontend is either Blazor or Avalonia (or MAUI or even WinForms if someone still wants to do those). In any case we can share a contract DLL => I want strongly typed in both directions, always. Doing stuff manually is only a potential for an error. If some dynamic logic is required it can still be stringly typed and this library is just not used.

Same thing about the possibility for reflection: this package is not used in legacy code (because it doesn’t really exist yet) and going forward when there is a source generation option I’d pick it – for better AOT compatibility alone. I can hardly think of a scenario where the amount of code generated for a simple SignalR client would be a concern regarding assembly size. Yet even if that is the case or more flexibility is required, we are still able to stringly type to our heart’s contents.

I’m sure you have very valid reasons for your current plans, but I wanted to voice my view of preferring less complexity in this case anyway. So, in my opinion supporting only scenario 3 and source generation would be sufficient. But you don’t have to hurry, there are good community solutions out there already => take your time to find a good canonical solution.

WeihanLi commented 11 months ago

Any plan to make it in .NET 9? think it would be greatly helpful

stephenstarkie commented 9 months ago

I have tried to use this to build strongly typed SignalR services - I have copied the pattern described here and in https://kristoffer-strube.dk/post/typed-signalr-clients-making-type-safe-real-time-communication-in-dotnet/ While the code from the GitHub repo in the post works, my own does not. I note that another user has also had the same problem on StackOverflow: https://stackoverflow.com/questions/77370046/signalr-source-generator-always-throw-exception (I notice the only answer there has not been marked as working and doesn't work for me). I notice that sometimes when I have two solutions open (mine and DistributedStickyNotes), the distributed sticky notes proxy can suddenly turn up in the autogenerated .g.cs in my solution, until I change something or rebuild in my solution. The only other difference is that I am trying to work with a serverless hub - but I can't see how a client side generator would know that.

DisitributedStickyNotes autogenerates this:

public static partial class HubConnectionExtensions
    {
        public static partial T ServerProxy<T>(this HubConnection connection)
        {

            if (typeof(T) == typeof(Shared.IStickyNoteHub))
            {
                return (T) (Shared.IStickyNoteHub) new Client.Proxies.HubConnectionExtensions.GeneratedIStickyNoteHub(connection);
            }
            throw new System.ArgumentException(nameof(T));
        }
    }

My code autogenerates this;

public static partial class HubConnectionExtensions
    {
        public static partial T ServerProxy<T>(this HubConnection connection)
        {

            throw new System.ArgumentException(nameof(T));
        }
    }
stephenstarkie commented 9 months ago

OK - so I figured out my problem; because I referenced the HubConnectionExtensions from a shared library into two apps: a Blazor app and a Windows Service the generator didn't work - as soon as I put the HubConnectionExtensions partial class in each of the two apps the source generators work