dotnet / AspNetCore.Docs

Documentation for ASP.NET Core
https://docs.microsoft.com/aspnet/core
Creative Commons Attribution 4.0 International
12.53k stars 25.3k forks source link

Additional JSImport/JSExport Examples #32001

Closed SerratedSharp closed 2 weeks ago

SerratedSharp commented 6 months ago

Description

Proposal and offer to produce a PR to include additional examples of using JSImport/JSExport.

1) Optionally, I would propose creating a new article focused on .NET/JS Interop.

2) Regardless of the above decision point, my primary ask is to add additional concrete usage examples of JSImport:

I would consider proposing some more advanced scenarios and possible troubleshooting section to address pitfalls for those new to JS interop, but there'd probably be some debate about some of those items so I'd defer to a future PR for the sake of getting agreement on the core ask here. Also I didn't cover anything JSExport related. Point being, I think there's room for it to grow.

Let me know a decision point on 1) and thoughts on 2), and I'd be happy to draft a PR.

Page URL

https://learn.microsoft.com/en-us/aspnet/core/client-side/dotnet-interop?view=aspnetcore-8.0

Content source URL

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/client-side/dotnet-interop.md

Document ID

efff6954-a035-d1da-5975-af04ff7c05ed

Article author

@pavelsavara

pavelsavara commented 6 months ago

cc @guardrex

pavelsavara commented 6 months ago

I have no opinion or preference on how to organize the topic in terms of pages.

Regarding the demo, when we introduced this interop, I wrote an article about it. And it contains links to few simple demos.

This covers some of the scenarios you mentioned https://github.com/pavelsavara/dotnet-wasm-todo-mvc/blob/main/View.cs#L36

We also have this demo in progress, which is about single-threading dotnet running in a webworker https://github.com/dotnet/runtime/issues/95452

And we will need a demo for multi-threading in Net9 and how it interacts with JS.

As personal opinion, I don't promote exposing OOP across the interop boundary. So I would not recommend showing people how to create wrappers around JS classes and passing around their proxies. Because usually interacting with things like instances of DOM elements individually creates very chatty interaction over the interop boundary and requires to allocate many proxies and even callbacks. It's slow and resource expensive.

My recommended pattern is to use JS to group functionality into Facade and call that via interop in more coarse-grained style , where the interop API is expressing business intention rather than technical implementation.

SerratedSharp commented 6 months ago

[...] I would not recommend showing people how to create wrappers around JS classes and passing around their proxies. Because usually interacting with things like instances of DOM elements individually creates very chatty interaction over the interop boundary and requires to allocate many proxies and even callbacks. It's slow and resource expensive.

My recommended pattern is to use JS to group functionality into Facade and call that via interop in more coarse-grained style , where the interop API is expressing business intention rather than technical implementation.

I think we can demo the capability, but temper its usage by including notes on its costs?

guardrex commented 4 months ago

@mkArtakMSFT and @danroth27 will need to decide if this will be pursued.

Note on splitting common content (which is currently handled by an INCLUDE file for the type mapping piece) ... I don't see how that's going to work with our ToC, where Blazor articles are generally self-contained documents under the Blazor node and client-side development is a different node SxS with the Blazor node. I think the approach that we've currently embraced is good under the circumstances, but we can discuss it further here.

In case Artak and Dan don't see the ping here, I'll email them now to ask them to take a look at this issue and decide if/how we'll proceed.

danroth27 commented 4 months ago

I would propose creating a new article focused on .NET/JS Interop.

I do like the idea of pull out the JSImport/JSExport content into its own article that then gets referenced from the existingJavaScript JSImport/JSExport interop with ASP.NET Core Blazor and Run .NET from JavaScript articles. @SerratedSharp What title and table of contents (TOC) location are you proposing?

Blazor articles are generally self-contained documents under the Blazor node and client-side development is a different node SxS with the Blazor node.

@guardrex Why is this a concern? Blazor articles can reference articles outside of the Blazor node, right?

Add additional concrete usage examples of JSImport:

The proposed samples look like goodness to me. I assume the goal here is to demonstrate the available mechanisms for JS interop with appropriate scenarios.

I would not recommend showing people how to create wrappers around JS classes and passing around their proxies.

@pavelsavara Presumably we can provide appropriate samples that demonstrate the available supported features without showing antipatterns? Or are you saying there are specific JS interop features that we simply should not document?

guardrex commented 4 months ago

Client-side development and Blazor are SxS nodes in the Web apps node. Those two articles reside across those nodes, so would the new common-content article go in the root Web apps node?

guardrex commented 4 months ago

That's part of the reason that the content is in an INCLUDE and appears in both articles. There's no need for a common-content article that only resides in one place breaking a linear reading order down either of these nodes. I also steered clear of only having this content in one or the other articles: Blazor devs don't need to see the client-side dev article, and client-side (non-Blazor) devs don't need to see the Blazor article. It seemed like the best way to handle it at the time.

danroth27 commented 4 months ago

@guardrex I see. I get your concern, but in this case, I think it's ok to compromise and live with the SxS relationship. The JSImport/JSExport content is more like a piece of the .NET runtime. We just happen to document it as part of the ASP.NET Core docs because it's most relevant to web developers (even though you can technically use the same functionality to run .NET code in any JS environment, like a Node.js process).

So, I think we just need to decide if the new JSImport/JSExport article should be a sibling of the existing Run .NET from JavaScript article or if it should be a child article. The title should presumably be something like "JavaScript interop with [JSImport]/[JSExport]" (no Blazor stuff). Would that work?

guardrex commented 4 months ago

It seems like doing this is only helpful under two conditions ...

If those two conditions aren't true, a silly green dinosaur πŸ¦–πŸ˜„ isn't sure this will be helpful.

... and @SerratedSharp ... your remark on the search result can probably be fixed by a mere title change to include "[JSImport]/[JSExport]" language in the title of the Client-side dev article. These titles could be ...

Anyway ... to proceed with a shake-up here with a common-content article, this is what I have thus far ...

The Blazor article, which lives in Blazor > JS interop node, would state at the top (or after Prereqs) to go read the common-content article over in the Client-side development node and lose the INCLUDE, which would take most of the title of the Blazor article sans "Blazor," and come back to the Blazor article to pick up with the rest of the Blazor-specific coverage. We need a cross-link back over to the Blazor doc because otherwise devs will probably lose their place when the Blazor node tree closes in the UI. The existing Run .NET from JavaScript article might be renamed, but it will also lose the INCLUDE and appear immediately after the new common-content article in the Client-side development node.

danroth27 commented 4 months ago

I thought the new factoring would be useful for non-Blazor app models that rely on our .NET WebAssembly support, like Uno and OpenSilver. Developers using these app models don't actually need to know how to set up a .NET app on WebAssembly - the app model has already taken care of that just like we do in Blazor. But the user may need to know how to do JS interop. The docs for these app models would presumably prefer to reference an article that is specific to JS interop that doesn't have the other stuff in it on how to set things up.

guardrex commented 4 months ago

I see. That makes sense. There's a another option to just stick with the same pattern ... create that new article for non-.NET WebAssembly support app models wherever it makes sense (Client-side dev node, too, probably) and place the INCLUDE into it as well.

Again tho ... the original concerns are good ones ...

guardrex commented 4 months ago

Another aspect to consider is that normally we show setup first (prereqs, tooling, app config, etc.), and then we talk about conceptual and reference matters. It sounds to me like the common article here has that in reverse with the JS interop content being consumed before the prereqs, tooling, app config, etc. in the individual articles. That's a bit strange for us, and I'm not sure how that's going to be received by readers πŸ€”. It doesn't adhere to general-to-specific and step-by-step ordering, both of which are heavily considered in every decision I've made for docs since ... well ... ASP.NET Core Preview Beta 5! πŸ‘΄πŸ˜†

danroth27 commented 4 months ago

It sounds to me like the common article here has that in reverse with the JS interop content being consumed before the prereqs, tooling, app config, etc. in the individual articles.

Why would someone end up reading the new JS interop article first before the setup content? I was thinking someone would end up on this article after reading about Blazor JS interop, or after reading about setting up .NET to run on WebAssembly, or after getting set up with some other app model that uses .NET on WebAssembly.

guardrex commented 4 months ago

I said that because I thought the common content was for what's in the INCLUDE ...

https://github.com/dotnet/AspNetCore.Docs/blob/main/aspnetcore/blazor/includes/js-interop/8.0/import-export-interop-mappings.md

... which appears in the middle of the current articles, and @SerratedSharp said about the existing articles that they would ...

keep some of their specialized minimal examples because they do a good job of tieing it all together and showing how the interop code fits into the application

... and I think @SerratedSharp is referring to content after the INCLUDE. So, I might be confused on what @SerratedSharp is proposing on ordering of articles and content in them. It sounded a bit like the common content would be pulled out and appear before these existing articles (+ a new one), then prereqs/setup/config (existing articles), and then the examples (existing articles).

An outline would be helpful, @SerratedSharp, of what you're proposing on ordering of articles and sections in them.

SerratedSharp commented 4 months ago

The existing articles have a fair amount of platform specific plumbing. IMO I would leave those articles as-is for the most part, and the intent is they give the dev just enough examples to help them wrap their head around how everything fits together. I.e. the blazor articles shows some JSInterop in the context of razor pages, the wasm-browser articles covers alot of important details on setup/plumbing and briefly touches on JSImport/Export, etc. At the end of the "Call JavaScript from .NET" section and "JavaScript interop on WASM" sections respectively, I would have some additional prompt "For additional examples of using System.Runtime.InteropServices.JavaScript for JS interop, see link to new article."

As others mentioned, the existing articles are more guides, where-as I would imagine the new JSImport/JSExport focused article to be more of a reference.

The new JSImport/JSExport article could start with a brief intro that backlinks to the other articles along the lines of "This code must be run from within a WebAssembly in the context of a JavaScript host, such as a browser. For details specific to Blazor, see blazor article. To setup a non-Blazor project using wasm-browser templates, see wasm-browser article". Some word-smithing is needed, but you get the idea. The two things I'm careful to convey here is "JS host" because there's other web assembly project types that aren't hosted in JS that JSImport/JSExport doesn't work in, and secondly the first statement is generalized because there's some other JS hosted WASM platforms where JSImport/JSExport works besides what you document, i.e. Uno.Bootstrap.Wasm.

I might consider moving the table of supported mappings to the reference article. It doesn't seem to fit in the theme of the how-to-guide style of those articles, and is more of a reference IMO. But that's just an after thought.

In summary:

The notes on prerequisites with backlinks should ensure if somehow a dev finds themselves in the reference article first, then they are prompted to go through one of the guides first.

guardrex commented 4 months ago

Ok ... nice. For .....

I might consider moving the table of supported mappings to the reference article. It doesn't seem to fit in the theme of the how-to-guide style of those articles, and is more of a reference IMO. But that's just an after thought.

I would suggest a passing mention and link to the supported mappings from the articles where they appear. Then, yes, strip that INCLUDE out and place that INCLUDE text into the new article. Drop the INCLUDE from the repo completely.

... and I still think that a title change is needed for the Run .NET from JavaScript article, possibly with a file name (URL) and UID update as well. I'll advise on that depending on your answer to the question .........................

From your OP, you're volunteering to take this on? We offer byline credit with a cross-link of choice ... well ... don't cross-link your name to The Nazi Party of Antarctica! πŸ˜† or anything horrible like that ... cross-link to your company, blog, Twitter (X ... whatever he calls it now πŸ˜„). We offer byline credit for community members who write entire articles for us, and it helps some folks with MVP status and/or street cred in the industry ... driving traffic to their product or service ... that sort of thing ... looks great on a resume for job seekers.

If so, I'll advise further on the approach to get everything in the right spot and deal with file names, UIDs, etc.

SerratedSharp commented 4 months ago

From your OP, you're volunteering to take this on?

I wanted to at least draft the majority of the article. I do alot of internal documentation and I will try to mimick existing documentation styles on MS, but I don't super want to get bogged down in word-smithing. So my thought was create the article/updates in my fork, then provide those and see what feedback there is. At that point if it seems like I'm way off base stylistically or there's alot of ceremony in getting it integrated, then if there was someone better equipped who wanted to take it over I don't really care that much about credits.

I started moving my examples into a wasm-browser project just to make sure they all work in that context as well, and adjusting some to be a little less opinionated, and also thinking about some of Pavel's feedback in that. I got side tracked in experimenting with some other wasm stuff while doing that and will also be on the road for a couple weeks, but can refocus back on this. I can target having something ready for review by May 18th.

guardrex commented 4 months ago

That's cool. You can even just place the text into an issue comment here, and I'll take it from there ... whatever is easiest/best. Yes, indeed! I can handle the 1,000 MS style manual rules 😩 + the 1,000 ASP.NET Core doc style rules 😩😩 + the additional 1,000 πŸ¦– Blazor doc style rules 😩😩😩.

SerratedSharp commented 3 months ago

@guardrex I created a PR into my own fork to gather initial feedback: https://github.com/SerratedSharp/AspNetCore.Docs/pull/1 @pavelsavara Pinging in case you want to review content in new article: https://github.com/SerratedSharp/AspNetCore.Docs/blob/serrated/32001JSImportExamples/aspnetcore/client-side/dotnet-js-interop.md

Let me know if it would be better to do a merge squash into my own fork to create a more concise commit for preliminary feedback. Once I have made updates based on feedback, I can either do a proper PR into this repo or leave it as a separate branch in my fork to be cannibalized.

guardrex commented 3 months ago

Thanks @SerratedSharp ... I'm not keen on the content organization; but if everyone else is happy, let's proceed. I'll make a quick remark on some quick styles that you can adopt on your branch and make the rest of the edits directly to the text over here after it becomes a PR on this repo. If you want me to create the PR over here, let me know. Otherwise, I'll react after you set it up. I can edit the files directly, and that's a big time-saver on editing the content. The important thing is just to get the file names correct, and I'll comment on that on your branch in a minute.

guardrex commented 3 months ago

UPDATE: Mentioned on @SerratedSharp's branch: Given the number of rules and conventions that we must follow for new articles, it's best (fastest πŸƒβ€β™‚οΈ) if I just take the content from there. I could explain it all out, but it would take a lot of time, and I'm already now getting behind on my tutorial work this week.

I should be free to work on this next week πŸ€žπŸ€. I'll drop a comment here if my schedule changes and it gets pushed back.

guardrex commented 3 months ago

@SerratedSharp ... I'll need info for the byline ...

By {YOUR NAME}

... with your name linked to your company, blog, or social media account. What do you want to link to (URL)?

SerratedSharp commented 3 months ago

Thanks @guardrex

By Aaron Shumaker (https://github.com/SerratedSharp)

guardrex commented 2 months ago

FYI on this one πŸ“£

I'll be OOF all of next week (6/24-6/28), and this week I've been busy with higher priority issues. This issue is at the top of my P2: Medium priority issue list, so it will be worked fairly soon. However, it will need to wait until I get back from next week's break. I hope to reach it the week of July 1, but that's if a bunch of new issues don't come in next week with a higher priority or other high priority tasks don't hit me (e.g., Pre6 is coming up for July 9). I'll get to this as soon as I can πŸƒβ€β™‚.

guardrex commented 1 month ago

UPDATE (7/10): I'm working my way to this, but I still have a couple of higher priority issues to resolve first. Part of the reason that it's taking so long is that .NET 9 season is upon us. All .NET 9 work has very high priority, and those issues are taking some time to resolve. I'll get to this issue as soon as I can, but it might take a few more weeks.

guardrex commented 1 month ago

UPDATE (7/22): Finally! πŸ˜… I've reached this issue for work. I guess we're going to use the "fork to be cannibalized" approach mentioned above :point_up:. I'll set up the PR from your fork, @SerratedSharp, and ping everyone from there.

guardrex commented 2 weeks ago

@SerratedSharp ...

πŸŽ‰πŸŽ†πŸŽˆ It's LIVE! πŸ•ΊπŸ’ƒπŸ» ...

https://learn.microsoft.com/en-us/aspnet/core/client-side/dotnet-interop/?view=aspnetcore-8.0

Thanks again! πŸ₯‡ Great article! I'm sure that devs will find it super helpful. If you ever need to change the byline link destination, shoot a PR in and ping me on it.

If reader feedback comes in for anything significant or anything that I need help with, I'll ping you on the opened issue.