Aspnetcore: Blazor API Review: JS Interop

[rynowak]

Some of my thoughts about this.... I'm going to stick to the .NET side because I'm not entirely certain what to optimize for in the TS apis.

DotNetDispatcher

I'd like to choose a name or namespace for dedicated interop types. Previously .NET has given the guidance to use specific names. This kind of thing is valuable because it helps organization and works with tooling like analyzers (can teach analyzers to ignore that namespace).

We're also planning to build an analyzer that will warn on usage of unsupported APIs (for ASP.NET Core as a whole). This is something we could leverage here as well - (don't call interop APIs from .NET).

I'm thinking about that because I look at some of these signatures, and I think they would break if we added more features. For instance, what about if we added support for generics? That would require adding another new API since we'd have no way to pass a type in there. Adding a parameter object would help.

Rather than trying to polish what's here to a mirror sheen, I think it's a better idea to declare this area as "implementation detail only".

JSAsyncCallResult should probably move to this namespace.

JSInvokableAttribute

JSInvokableAttribute - this should be sealed and inherited = true. I'm not sure why we decided on AllowMultiple? Is this to support multiple names per method? Unless we have a test for that I would be really surprised if it works.

DotNetObjectRef

We can write a custom converter to get rid of __dotNetObject.

We need to lock on the name of this. I'm not a big fan of seeing DotNet in names, it seems really not nice to read 😢 I'm also not a fan of Ref since we don't generally abbreviate in names.

Some suggestions:

• ObjectReference
• JSReference

I don't want to drill into this too hard, if there isn't another suggestion that we really like, then it should stay as is.

IJSRuntime and related types

We should consider using ValueTask<>. This would save an allocation in in-process scenarios.

I'm not totally certain why we have separate interfaces and abstract classes. My main concern is that we can't easily add members to the interfaces, so I'm not sure what exactly they buy us vs just the classes.

BeginInvokeJS could have its name improved, like making it consistent with InvokeAsync (remove the JS). Some names could be made less idiosyncratic in general in the implementations.

I'm not sure that my concerns about the security of InvokeAsync and the semantics of passing the default cancellation token were ever addressed.

OnDotNetInvocationException seems like a really fragile contract. It's not clear why the method and assembly are arguments here vs a parameter object. Imagine we added support for generics, this signature is broken now.

[pranavkm]

Throwing in JSMarshalByRefObject as a suggestion for the name

I wanted to consider removing the DotNetObjectRef.Create API and instead have an API on IJSRuntime \ JSRuntimeBase that exposed this. Right now, the Create API statically gets at the JSRuntimeBase instance. Having it be on the runtime makes it a bit more obvious that it's the one managing the lifetime of the reference and avoids creating spooky issues like https://github.com/aspnet/AspNetCore/issues/11159.

Proposal

C# JSMarshalByRefObject<T> IJSRuntime.CreateMarshalByRefObject(T value) where T : object

In addition to this, as part of the Dispatcher's argument binding, we should also consider binding IJSRuntime instances with the current \ correct value. This would prevent users from having to figure this out themselves in case they need to call back in to the IJSRuntime as also required if we decide to go with my first suggestion. You will need an instance of IJSRuntime to do this: https://github.com/aspnet/AspNetCore/blob/87a92e52c8b4bb7cb75ff78d53d641b1d34f8775/src/Components/test/testassets/BasicTestApp/ServerReliability/JSInterop.cs#L8-L12

[pranavkm]

Another follow: calls to DotNetDispatcher typically involve looking up the instance of JSRuntimeBase using the JSRuntime.Current AsyncLocal instance. Could we consider a model where the runtime is passed in rather than relying on ambient state? Makes the API much more explicit and easier to reason about:

public static partial class DotNetDispatcher
{
    public static void BeginInvoke(JSRuntimeBase jsRuntimeBase, string callId, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) { }

    public static void EndInvoke(JSRuntimeBase jsRuntimeBase, string arguments) { }

    public static string Invoke(JSRuntimeBase jsRuntimeBase, string assemblyName, string methodIdentifier, long dotNetObjectId, string argsJson) { throw null; }


    [Microsoft.JSInterop.JSInvokableAttribute("DotNetDispatcher.ReleaseDotNetObject")]
    public static void ReleaseDotNetObject(JSRuntimeBase jsRuntimeBase, long dotNetObjectId) { }
}

The only interesting behavior to add would for JSInvokable methods. We would "bind" the JSRuntime instance to a parameter if it's assignable from JSRuntimeBase, kinda how https://github.com/aspnet/Extensions/blob/master/src/JSInterop/Microsoft.JSInterop/src/DotNetDispatcher.cs#L200-L208 currently works.

[rynowak]

I caught up with @terrajobst to talk about the methods on IJSRuntime and our current POR. His thoughts were basically:

• Having two methods on the interface are fine, it's probably the best way to get what we want, and is appropriate because there are far more callers than implementors
• Make the CancellationToken the second parameter because a string is far more likely to appear in the list of parameters passed to JS than a cancellation token.
• Our lives will be simpler if we don't need the IEnumerable<object> family of methods, and are OK just using params object[].

As an example of this last one, consider what happens when you need to pass a List<Customer> as a single parameter to JS. That will convert to IEnumerable<object>. So any time someone is calling a JS method with a single parameter, and that parameter is a collection, they need to know about this quirk.

If we just do object[] then this case only occurs with arrays, which are less common. There's no great mitigation for this all-up.

[rynowak]

We also talked about using Span, but I'm not sure that applies because this is an async method. I think we're probably smart enough to implement this combo, but I think about people mocking the API and stuff.

[terrajobst]

but I think about people mocking the API and stuff.

That's a fair, but if that's a concern we could probably provide an implementation for mocking that takes a delegate. I'd say it depends on how valuable you see the reduction of allocations for JS interop.

[egil]

@rynowak, I do not know if its too late to provide input here, but ill try anyway :)

When building component libraries, we do not know if the user will be using them in a blazor client or blazor server scenario, and if the client will have prerenderinger enabled.

That means we must guard against a disconnected state, inject IComponentContext into our components and services, and check with IComponentContext.IsConnected before calling our JavaScript functions.

For components, it looks as if we can skip the JS call if IsConnected = false, since the components will get re-rendered again when the connection is established and then we get a second chance to call. E.g. the pattern looks a bit like this:

[Inject] private IComponentContext? Context { get; set; }

protected override Task OnAfterRenderAsync()
{
    if (!Context.IsConnected)
        return Task.CompletedTask;
    else
        return JsRuntime!.InvokeAsync<object>("jsFunc", ...);
}

For services (scoped/singleton) it is a bit more tricky as far as I can tell, since the service only gets instantiated once (during first render). Thus, if we have a DoSomethingAsync() method that uses JS and it can be called by a component, we must spin in the service, waiting for the connection to be established. E.g.:

public class MyService : IDisposable
{
    private readonly IJSRuntime _jsRuntime;
    private readonly IComponentContext _componentContext;
    private CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource();

    public MyService(IJSRuntime jsRuntime, IComponentContext componentContext)
    {
        _jsRuntime = jsRuntime;
        _componentContext = componentContext;
    }

    public async Task DoSomethingAsync()
    {
        var isConnected = await _componentContext.IsConnectedAsync(_cancellationTokenSource.Token);
        if (isConnected)
        {
            await _jsRuntime.InvokeAsync<object>("jsFunc", Array.Empty<object>(), _cancellationTokenSource.Token);
        }
    }

    public void Dispose()
    {
        _cancellationTokenSource.Cancel();
        _cancellationTokenSource.Dispose();
        _cancellationTokenSource = null;
    }
}

The above code is possible with this extension method:

public static class ComponentContextExtensions
{
    public static async Task<bool> IsConnectedAsync(this IComponentContext componentContext, CancellationToken token = default)
    {
        while (!componentContext.IsConnected && !token.IsCancellationRequested)
        {
            await Task.Delay(50, token);
        }
        return componentContext.IsConnected;
    }
}

The above pattern for services is probably not always needed. I have one case where I think it is though: I have a service that subscribes to particular DOM events (Page Visibility API) when the first component subscribes to it. That way, one or more components needing to know about that DOM event only results in one subscription on the DOM. However, if IsConnected is false, it spins as shown in the example above, and subscribes to the DOM event as soon as it is possible. The component thus doesn't have to deal with connected or disconnected states, which is nice, since they are just subscribing to the service using regular .net events.

To sum up, I think the patterns I've shown above are OK, but it would be nice if there was an easier way, perhaps with a little more help from the JSRuntime/ComponentContext APIs, to deal with the connected/disconnected scenarios.

[rynowak]

Thanks for reaching out. The following is out of date:

When building component libraries, we do not know if the user will be using them in a blazor client or blazor server scenario, and if the client will have prerenderinger enabled. That means we must guard against a disconnected state, inject IComponentContext into our components and services, and check with IComponentContext.IsConnected before calling our JavaScript function

OnAfterRender is no longer called in prerendering. We're actually removing IComponentContext because we've solved this problem by just making OnAfterRender fit for purpose.

Thus, if we have a DoSomethingAsync() method that uses JS and it can be called by a component, we must spin in the service, waiting for the connection to be established

I'd suggest passing this problem to your caller. Code that calls into JS interop is going to have to be prepared for exceptions coming from the JS side. It's not expected that JS interop is bulletproof - because the JS side is untrusted code, and the connection is not guaranteed to have 100% uptime.

[egil]

OnAfterRender is no longer called in prerendering. We're actually removing IComponentContext because we've solved this problem by just making OnAfterRender fit for purpose.

Is that in P8? I just did a quick dotnet new blazorserver -o test, added a <Test /> component to the Shared folder with the following content:

@code {
  protected override void OnAfterRender() {
    Console.WriteLine("Test.OnAfterRender");
  }
}
<h5>Hello from Test</h5>

When the <Test /> component is included in Index.razor and I do a dotnet run and browse to it, I see two calls to Test.OnAfterRender. If I remove prerendering from _Hosts.cshtml, I only see one call.

I'd suggest passing this problem to your caller. Code that calls into JS interop is going to have to be prepared for exceptions coming from the JS side. It's not expected that JS interop is bulletproof - because the JS side is untrusted code, and the connection is not guaranteed to have 100% uptime.

I am not sure I agree with this. It seems like my service would be violating a bunch of SOLID principles, if all the users of my service have to deal with possible JS interop code. It seems better to me to have the service take responsibility and shield the users as much as possible, and in that sense, giving the service the possibility to detect when the JS connection is down is not a bad idea. Especially in scenarios as the one I tried to describe in the end of my previous post.

Thus, I hope you will continue to provide a way for services to detect if JS interop is present and available, and at least provide some guidance on how and when to the connection can be dropped, when a service should give up on it entirely, and when it might come back.

[julienGrd]

Hi guys, i still have the error "JSRuntime must be set up correctly and must be an instance of JSRuntimeBase to use DotNetObjectRef.'" when i try to inject component in js. I don't know if it still "normal" with preview 8 ?
protected override async Task OnAfterRenderAsync() { if (ComponentContext.IsConnected) { try { await this.InvokeJsAsync<object>("injectModalManager", DotNetObjectRef.Create(this)); } catch (Exception ex) { //https://github.com/aspnet/AspNetCore/issues/11159 } } }

thanks !

[egil]

Hi guys, i still have the error "JSRuntime must be set up correctly and must be an instance of JSRuntimeBase to use DotNetObjectRef.'" when i try to inject component in js. I don't know if it still "normal" with preview 8 ?

Ryan writes in the related issue that it will be solved when this is issue is resolved, so the answer is yes.

[rynowak]

Is that in P8?

This is preview 9. I just checked with an example similar to yours to make sure we got it right.

Unfortunately the team is working a few weeks ahead of the public release right now. Preview 8 just shipped this week, but as of today we're done with preview 9 changes 😆 I realize that's confusing to follow along with.

I am not sure I agree with this. It seems like my service would be violating a bunch of SOLID principles, if all the users of my service have to deal with possible JS interop code. It seems better to me to have the service take responsibility and shield the users as much as possible, and in that sense, giving the service the possibility to detect when the JS connection is down is not a bad idea. Especially in scenarios as the one I tried to describe in the end of my previous post.

I realize that may not be convenient but it's unavoidable. Any time you have a pattern like if (connected) { DoStuff(); } you have the possibility of a race condition blowing things up on. Hopefully these kinds of problems are rare in practice.

You also have to keep in mind that JS interop runs arbitrary code on the client. Your client might be running JS code that you didn't write. A hostile client might stub out some JS call to never return, or return bad results. Basically anywhere you're using JS interop, you're going to want to harden that, including tasks that get cancelled or connection failures.

Thus, I hope you will continue to provide a way for services to detect if JS interop is present and available, and at least provide some guidance on how and when to the connection can be dropped, when a service should give up on it entirely, and when it might come back.

Can you log a new issue and include some examples (including how you want to handle disconnected cases in your APIs)? I'm happy to continue a discussion on this topic, and do more work in the future to enable you. I suspect that IComponentContext ultimately would have been too primitive for your needs.

[rynowak]

@julienGrd - please log a new issue and don't piggyback on the this API review discussion 😆

[pranavkm]

Done.

[egil]

Can you log a new issue and include some examples (including how you want to handle disconnected cases in your APIs)? I'm happy to continue a discussion on this topic, and do more work in the future to enable you. I suspect that IComponentContext ultimately would have been too primitive for your needs.

Thanks. I will see what is possible and what is not when P9/RC lands. Then, I might take you up on that offer. Thanks again.

[rynowak]

Thanks. I will see what is possible and what is not when P9/RC lands. Then, I might take you up on that offer.

Cool, what you should expect is that these scenarios have simpler guidance:

• OnAfterRender is not called in prerendering
• There's a firstRender parameter to make it more idoimatic to initialize JS Interop for a component
• IComponentContext is removed

The main reason we added IComponentContext was to make it possible for a component to know if it was in prerendering or not. The fact that it exposed whether or not you were connected was a detail that came up during implementation as the most sensible way to describe the API. I think it's problematic for the kinds of use cases you described, because it doesn't expose eventing or asynchrony - there's no way to be notified when the state changes.

The best course of action for us was to remove it for the 3.0 release, because we didn't really think it offered any value.

[egil]

OK @rynowak, let me share my current thoughts, maybe you can fill in a few gaps :)

My scenario is primarily a component library/service library setting. Ideally, since Razor components leverages dependency injection, the users of my library doesn't need to know or should know whether or not the services or components in my library uses JSInterop. If they do, they would, like you say, have to be aware and handle of the various pitfalls with JSInterop.

If I later remove a components or services dependency on JSInterop (you guys might release new APIs later that means I don't need it), then all my users who have been good Razor citizens and added proper error handling for JSInterop, they would likely need to clean up their code again.

With the above in mind, I will as a library author always prefer to handle the _expected_ JSInterop errors, and wrap the _unexpected_ JSInterop errors in a custom exception type, that generally signal to users of my library that something unexpected happened. That way, the users of my libraries can have a more simplified error handling, or none at all, and at least one that is less likely to change.

Known or expected errors

This is where you can probably help -- what are the all the things that can go wrong in JSInterop world, and how to recognize each one?

1. _Prerendering - JSInterop is not available_. Most likely cause of action is to simple continue and not do anything.
2. _JSInterop call doesn't go through or doesn't complete_. Cause of action depends on scenario, but a circuit breaker approach could be a possibility in some cases. At least retry N times with X delay.
3. _Called JS function doesn't exist_ (variant of 2. option). Cause of action could probably try again after a short delay, since browser might not have completed downloading/parsing the source javascript file, then after a little while throw an exception
4. _Mangled response_. Cause of action is very dependent on context. If we worry about security implications due to the scenario, then maybe throw an cannot continue exception and have the user of the service or component decide what to do. If it is more benign, like my case with the Page Visibility API, then maybe simple ignore the response and wait for a next one.

There might be other considerations for client side blazor, and I cannot claim to have any experience with SingalR directly, so there might also be some protocol-level errors that can be handled gracefully, depending on scenario/context.

Thoughts?