Skip to content
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

New Blazor JS interop #27016

Closed
guardrex opened this issue Sep 15, 2022 · 21 comments · Fixed by #27311
Closed

New Blazor JS interop #27016

guardrex opened this issue Sep 15, 2022 · 21 comments · Fixed by #27311

Comments

@guardrex
Copy link
Collaborator

guardrex commented Sep 15, 2022

Per dotnet/blazor-samples#41 ...

there is new JavaScript interop available for .Net 7.

I created simple demo to show the new JavaScript interop in Blazor.

I'm not sure if you want to adopt it as is, but perhaps you could have look at it and extend one of the existing sample apps.

https://github.com/pavelsavara/blazor-wasm-hands-pose

There is also non-Blazor demo which has more complex interop logic

https://github.com/pavelsavara/dotnet-wasm-todo-mvc

(also please ping me on IM to discuss how to collaborate on further docs about the feature, thank you)

It's best if we have a section and extend the snippet sample apps, as opposed to adding a new sample app. Every sample app adds overhead costs 💸☹️.

cc: @pavelsavara


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@dotnet-bot dotnet-bot added Blazor Source - Docs.ms Docs Customer feedback via GitHub Issue labels Sep 15, 2022
@guardrex guardrex added doc-enhancement Pri1 and removed Source - Docs.ms Docs Customer feedback via GitHub Issue labels Sep 15, 2022
@guardrex

This comment was marked as resolved.

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 15, 2022

@pavelsavara ... Ok ... Came back this afternoon to try again, and I had better luck 🍀 ...

  1. <AllowUnsafeBlocks>true</AllowUnsafeBlocks> in the project file.

  2. wwwroot/js/scripts.js:

    export function jsInteropCall() {
      return 'Hello World!';
    }
  3. RegisteredComponentsInterop.cs:

    using System.Runtime.InteropServices.JavaScript;
    
    internal sealed partial class RegisteredComponentsInterop
    {
        [JSImport("jsInteropCall", "Interop")]
        public static partial string JSInteropCall();
    }
  4. Pages/Index.razor:

    @page "/"
    @using System.Runtime.InteropServices.JavaScript
    
    <p>
        value: @value
    </p>
    
    @code {
        private string? value;
    
        protected override async Task OnAfterRenderAsync(bool firstRender)
        {
            if (firstRender)
            {
                await JSHost.ImportAsync("Interop", "../js/scripts.js");
                value = RegisteredComponentsInterop.JSInteropCall();
                StateHasChanged();
            }
        }
    }

... and that seems to work 🎉.

Lot's of ❓tho about what devs need to read on it ... best practices and critical info on things like ...

... stuff like that. Whatever you feel is the best general approach for the most common basic use case.

@pavelsavara
Copy link
Member

pavelsavara commented Sep 16, 2022

2. `wwwroot/js/scripts.js`:

All previous options how Blazor could deploy the JS file are still there. My favorite is next to the razor component.

   ```
       protected override async Task OnAfterRenderAsync(bool firstRender)
               await JSHost.ImportAsync("RegisteredComponentsInterop", "../js/scripts.js");
   ```

I'm not really Blazor expert yet, but this seems that the script download would be delayed too late and slow down the first render with the JS code impact. Is there event which could do it earlier ?

* First of all, is this even unmarshalled interop at all???? ... or is this just ✨ **_NEW and IMPROVED_** ✨ JS interop? If that's the case, then we'll need to discuss what the actual replacement is for the **unmarshalled** approach with the new API, if that's even a thing any longer. My first goal for the updates is to deal with the two sections we have for unmarshalled interop at ...

This is marshaled interop and it's so fast you could replace any legacy unmarshaled interop with it. And you should because IJSUnmarshalledRuntime is now obsolete.
@danroth27 would be better guide to help you to express Blazor oficial position and wording on this.

* `<AllowUnsafeBlocks>` ... Security concerns?

It means that (user and) the code generator in Roslyn compiler could use pointers. Not a security concern. Rather developer would not be prevented from using lower level concepts in that assembly.

* You want `.mjs`? ... or is just `.js` ok in this context?

.js is ok on the web. It makes difference on NodeJS, which you are not targeting with Blazor.

* Anything in particular to say about the module and its exports for this approach?

It's standard ES6 module. The module name in the JSImport and ImportAsync should match each other and they need to be unique in the app. Therefore if you are authoring NuGet package, you should use your namespace/prefix.

* Naming remarks on the class and module (e.g., where I just placed `RegisteredComponentsInterop` to get this working)?

So far I did my as public static class Interop nested in the real class which is using it.

* Do you like using `OnAfterRender` for module import, or would initialization be better?

JS download takes time.
As early as possible. Maybe non-blocking async cal in parallel with other work.
Developers could also prefetch it.

@pavelsavara
Copy link
Member

Link to ES6 modules doc, you could probably even link it from our docs https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules

@pavelsavara
Copy link
Member

Also the new API is only available in the browser (not on server side). The code analyzer would complain about it with CA1416.
The correct way how to solve it is to use

if (OperatingSystem.IsBrowser())
{
    //only call the new interop from here
}

@pavelsavara
Copy link
Member

  • There is also JSExport which you should explore.
  • Could should show how to pass callback on JSImport method and how to register DOM event.

@pavelsavara
Copy link
Member

pavelsavara commented Sep 16, 2022

I don't know what was Blazor previous guidance on Blazor component instances together with JSInProcessRuntime calls. For example when you have grid on the page with multiple rows. Each row is Blazor component with the JS interop. How does the DOM event handler hit the correct C# instance of the row ?

  • With this new interop we could marshal C# instance during registration and pass it back on the JSExport call
  • We could also capture it in C# closure of the callback passed to JSImport call
  • We could use existing ways how Blazor does this with JSInProcessRuntime - I don't know what it is, sorry

@pavelsavara
Copy link
Member

  * https://docs.microsoft.com/en-us/aspnet/core/blazor/performance?view=aspnetcore-6.0#consider-the-use-of-unmarshalled-calls
  * https://docs.microsoft.com/en-us/aspnet/core/blazor/javascript-interoperability/call-javascript-from-dotnet?view=aspnetcore-6.0#unmarshalled-javascript-interop

Ideally these 2 articles would not be part of Net 7 because we don't want people to use it for new code. The JS APIs on BINDING and MONO objects are now also deprecated. They are also un-fixably unsafe during GC.

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 16, 2022

My favorite is next to the razor component.

We cover that, too. I'll be sure to link our module coverage in the new sections. One thing that I need to discuss with Dan and Artak is how we still favor the easy/quick placement of scripts in the index.html/_Host.cshtml files. It's good for doc examples, but it really isn't the best practice. We might favor going with modules everywhere. We have at least a dozen updates to make across the whole Blazor doc set to pull that off. However, I'll ask about it. If they want to do that, I can take it on near EOY or 23Q1. For now, we can go with modules in JS companion files to the components. I'll angle the PR that way.

the script download would be delayed too late and slow down the first render

It won't because it's after render (hence the need to call StateHasChanged in that hacked example I wrote about). Steve provided an example with Mapbox, and he says that it's good in cases where the dev has to interact with the UI ... i.e. ... wait until Blazor is done rendering on the first pass, then go for module/JS FNs that interact with the UI. But to answer your question, Yes! We can go with init. We'll see on the PR if Javier/Tanay/Steve confirm that it should say that when interacting with the UI that OnAfterRender is better. I'll go with init but make a remark about it in the text.

This is marshaled interop and it's so fast you could replace any legacy unmarshaled interop with it. And you should because IJSUnmarshalledRuntime is now obsolete.

This means that we can just drop the existing unmarshalled interop coverage from the 7.0 and later docs in favor of two new sections on the new interop approaches. That's cool. 👍

@danroth27 would be better guide to help you to express Blazor oficial position and wording on this.

No need at this point. I'm aware of how the draft should be assembled. We'll get feedback from Dan on the PR later.

I'll work up a 2nd example the other way with [JSExport].

As for the rest, LGTM. I think I have enough to draft the PR now. I'll ping u back here if I have more ❓.

I'm about out of hours ... and 🧠 power 😩 ... for the week. I'm going to pick back up with this on Monday morning, and I think the PR would will be ready no later than Wednesday of next week, as long as nothing high priority hits me in the meantime ⛰️⛏️😅.

@guardrex
Copy link
Collaborator Author

I see that you added a few more comments. I'll get back to you. I'm going OOF for a few hours, but I'll be back online later today ... and then again on Monday.

@guardrex
Copy link
Collaborator Author

I'm BACK. 🏃‍♂️🏃‍♂️🏃‍♂️🏃‍♂️

previous guidance on Blazor component instances together with JSInProcessRuntime calls.

Not recommended for general use because it isn't hosting model agnostic. I think 🤔 we only cover it in one spot at ...

https://docs.microsoft.com/en-us/aspnet/core/blazor/javascript-interoperability/call-javascript-from-dotnet?view=aspnetcore-6.0#synchronous-js-interop-in-blazor-webassembly-apps

For the use cases you mentioned in your bullet points, we'd cover basic use cases but probably not advanced ones ... probably not initially anyway. I'll put the simple, basic use cases on the PR, then we can see what the team thinks about additional use cases to add.

these 2 articles would not be part of Net 7

I'm happy to hear that 👍. That's easy. We'll be swapping the old sections for the new bits. What I'll probably do is keep a section and/or NOTE for one or two release cycles (.NET 7 to .NET 8 or 9) that tells readers about how that API is obsolete and recommends the new bits with a cross-link. If you put up an aspnet/Announcements issue on the obsolete API, I'll cross-link that. At .NET 8 (or 9), I'd pull the section/NOTE for future docs. We don't need to keep info on old bits forever.

I'm feel'in good about this. I think I have enough to get the PR set up. I think it will be up on ... mmmm 🤔 ... Wednesday/Thursday. I'll work on it locally for a few days before putting up something that I'm not embarrassed to show to the PU 🙈😆. If I run into trouble 😈, I'll shoot you a message here. Otherwise, I'll ping u on the PR next week.

@pavelsavara
Copy link
Member

Note to self:

  • add list of all marshaled types
  • add details about working with buffers without copy

@guardrex
Copy link
Collaborator Author

... and a quick update. I've been held up just a little bit by new Blazor issues, but the JS interop work is proceeding. I have the code examples for the PR. I plan to start writing the section today (Tuesday). I still think it will be up by Thursday for review. It will be helpful if fewer Blazor issues come in, but you know how that goes. I can go weeks without a peep from the readership, and then get slammed with a dozen issues in just a couple of days 🏃‍♂️😅.

@guardrex
Copy link
Collaborator Author

Another PR + a gRPC review (#27053) cut into my writing time today. It's looking more like Friday for the PR. Sorry for the delays ... it's a very 🏃‍♂️🏃‍♂️🏃‍♂️🏃‍♂️🏃‍♂️😅 time of year!

@guardrex
Copy link
Collaborator Author

@pavelsavara ... Update: It's going well. I think the PR will be up on Friday morning.

Pavel, I see that the the API and the syntactic approach ([JSImport]/[JSExport]) is for WASM at this time. I also think I see that significant parts of the framework will change over its internal JS interop to use the new API for .NET 8. What's not clear is if the general syntax and approach will ever be mirrored for Blazor Server JS interop. I don't mean the underlying mechanisms that the framework would use to make it work, I just mean from the standpoint of how developers implement JS interop in their Blazor Server apps. Can you let me know a little more about the long-range plan for this API approach (the dev implementation of JS interop in their apps)?

Otherwise ...

It seems more reasonable to create a new temporary for .NET 7 article to cover this for a couple of reasons:

  • I assume that at least for WASM that all of the Blazor docs and examples will adopt the new API for .NET 8 because the framework itself seems like it's headed that direction.
  • There's no new name for this API: It's just "new JS interop," which is much easier to cover SxS with the existing coverage in a new topic, not by trying to shoehorn the coverage into the existing coverage. Note that we can still do that tho ... we can still just create new sections in the existing Call JS from .NET and Call .NET from JS topics. Let's see on the first draft if using a new article just for .NET 7 makes sense with the long-range plan to move the coverage into the existing articles and dropping this new topic at .NET 8.

@pavelsavara
Copy link
Member

This is question for @danroth27 . Technically it would need it's own Roslyn code generator and the low level behavior of the marshaler would be different as it's a remote protocol via JSON. It would be another attribute. Somebody would need to do a prototype to see if it's good idea. I don't know.

Also do we commit our future plans in the documentation ?

The "new JS interop" is rather the "JS interop" from runtime perspective, because the previous API was private and undocumented.
From runtime perspective there is nothing temporary about it. Maybe we need to separate what is Blazor specific and what is general runtime into separate article for runtime ?

I work on an article for dotnet blog which describes many details of it. Do you have access to https://github.com/microsoft/dotnet-blog/pull/903 ?

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 22, 2022

Also do we commit our future plans in the documentation ?

No, only what's available, and we don't usually say in one version of the docs what's going on in another version. For example, we would rarely tell devs in a 6.0 doc, 'if you update to 7.0, you can do this better with new 7.0 bits.' It's too much work to do that constantly across the docs and for new releases. What we do is cover everything in the What's New doc for a given release and cross-link the new doc sections there. In the Migration topic we'd cover aspects like this, too. For example, we'll say that unmarshalled interop is obsolete and cross-link the new coverage for devs to migrate to 7.0.

nothing temporary about it

I mean just for coverage. I just mean that a new topic in the JS interop folder SxS would be a good temporary bridge to JS interop changing (perhaps only ever for WASM) to use [JSImport]/[JSExport] in .NET 8. I can't tell from your, @danroth27, blog post where this is headed for Blazor WASM JS interop insofar as the general coverage is concerned.

separate what is Blazor specific and what is general runtime into separate article for runtime ?

That sounds right to me 👂 because we focus on Blazor things here almost exclusively. However, @danroth27 will inform me if that's not going to be the case ... and then if not ... how the coverage should be laid out in the Blazor node (or another node perhaps). I'll be 👂 for that guidance.

In the meantime tho, I can at least get the Blazor-specific pieces put together by Friday so that we have a full explanation of the basics, and then you and Dan can see what else you'd like to change and add to that.

Do you have access to https://github.com/microsoft/dotnet-blog/pull/903 ?

It 404s for me.

@guardrex
Copy link
Collaborator Author

UPDATE (Thursday, 2pm CST): Looking good! 👍 I have a first draft now. I'll sleep 🛌💤 on it, make a final round of updates on Friday morning, and have it up in a PR no later than mid-morning (11am CST).

If you're going OOF for the weekend and want to hit it on Monday, that's cool. I'll be going OOF myself Friday afternoon for the weekend and back on Monday. I don't think we're in a super-rush for this given that we're still only in RC1 times.

It will be a bit rough, no doubt with some broken technical remarks to address 🙈 and lacking aspects that you'll want to add to it, including perhaps additional examples, but it should get us going in the right direction.

If it turns out that a new article for this in the JS interop folder SxS with current coverage isn't the best approach, no problem. This article breaks down cleanly into ...

  • Introductory content that can easily slip into the JS interop Overview article.
  • A call JS from .NET bit that can go into the existing Call JS from .NET article.
  • A call .NET from JS bit that can go into the Call .NET from JS article.

My recommendation is that you and I should make this content as nice as we can before pinging Dan and Artak on the PR. That way, they won't get pinged to death 🗣️💀 with a lot of back-and-forth discussion if we need to do a lot of work to it.

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 23, 2022

I wonder if the "new" JS interop could be named ...

.NET JavaScript Import/Export Interop

... to distinguish it from the existing interop (as a proper noun).

.NET JavaScript [JSImport]/[JSExport] Interop

... but that seems a bit unwieldy, and the attributes wouldn't localize.

If it can be distinguished in some way like that, it would be easy to add it to the existing topics (now or later) without causing confusion or requiring unusual adjectives like "new" or "alternative" JS interop.

UPDATE: Changed my mind. I think Dan would prefer to use the attributes after all, so I'll float it that way on the PR (with a lowercase "i" in "interop" ...

.NET JavaScript [JSImport]/[JSExport] interop

@guardrex
Copy link
Collaborator Author

guardrex commented Sep 23, 2022

@pavelsavara ...

For an Interop class ... e.g. ...

[SupportedOSPlatform("browser")]
public partial class Interop
{
    [JSImport("getMessage", "Interop")]
    internal static partial string GetWelcomeMessage();
}

... IIRC, you had [SupportedOSPlatform("browser")] on there in the framework PR but commented out ... is that needed ... or should I take that off because it's a no-op?

Nevermind! ... I see it in the sample app. What I'm recalling is probably from a framework test, where it probably would break the test.

@Aloento
Copy link

Aloento commented May 16, 2024

Why JSImport not support Blazor Server?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants