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

Prohibition on <script> tags for modules #34432

Merged
merged 7 commits into from
Jan 7, 2025

Conversation

guardrex
Copy link
Collaborator

@guardrex guardrex commented Jan 2, 2025

Fixes #34414

Thanks @gregoryagu! 🚀 ... I won't merge this until tomorrow morning (Friday) to give you a chance to review it.


Internal previews

📄 File 🔗 Preview link
aspnetcore/blazor/hosting-models.md aspnetcore/blazor/hosting-models
aspnetcore/blazor/javascript-interoperability/call-javascript-from-dotnet.md aspnetcore/blazor/javascript-interoperability/call-javascript-from-dotnet

@guardrex guardrex self-assigned this Jan 2, 2025
@guardrex
Copy link
Collaborator Author

guardrex commented Jan 3, 2025

@gregoryagu ... I think I'll need to do a little more work ...

  • The import remark should be cross-linked, and I think that's a dynamic import() call, so I'll place that cross-link to MDN as a guess.
  • Where I'm stating 'it won't load the module,' I think it's going to have to state somehow that that's merely because type="module" isn't in the <script> tag. Can you clarify from your testing if you placed the <script> tag with type="module" or not because I think if you have that it should load the script (from cache) when the dynamic import is executed by Blazor.

@guardrex
Copy link
Collaborator Author

guardrex commented Jan 3, 2025

I think on the second point that it's best to remove the remark. I really think that you can probably add a <script> tag for the module IF you properly declare type="module". It's a guess, but it seems like a good bet. The module would be loaded and cached (and slow down app startup). When the component invokes the import, the module should load from cache ... based on what I've heard anyway 😄.

I'll email Steve and ask him a few questions on these points. Done! 👍 Message sent. The PR seems like a good approach for coverage, but I'll wait to hear back from Steve on a couple of these points before merging.

@gregoryagu
Copy link

Sorry for the late reply, I am just getting to my emails.

I did a little repro to verify. Here is what is shows:

If you usethis.JSModule = await JS.InvokeAsync<IJSObjectReference>("import", "./Components/Pages/Counter.razor.js"); then there is no need to add a <script> tag in app.razor.

If you do add a script tag, then the file is downloaded twice, once when the app starts, and then once when you navigate to the page.

Thus there is no need to add the script tag unless you purposely wanted it loaded up front.

Here is the Console window in the browser that shows it was downloaded and executed twice:
image

However, as you point out, if you add type="module" then it is only loaded the first time.

<script type="module" src="/Components/Pages/Counter.razor.js"></script>

This is the Repository

@gregoryagu
Copy link

Here is how I would write it:

Don't place a <script> tag for JsCollocation2.razor.js after the Blazor script because the module is loaded and cached automatically. Adding a <script> tag causes the module to be loaded twice unless type="module is added to the script tag. Then it is only loaded from the script.

@guardrex
Copy link
Collaborator Author

guardrex commented Jan 3, 2025

Thanks for the additional investigation.

if you add type="module" then it is only loaded the first time

Good! My hunch was correct 😄, and that moves the latency to app startup, which most people probably wouldn't want.

Adding a <script> tag causes the module to be loaded twice unless type="module" is added to the script tag. Then it is only loaded from the script.

I'll add something along those lines. I'd like to mention the bit about front-loading the latency in case someone really wants to do that, along the lines of ...

... loaded from the <script> tag at app startup, moving the module loading latency from the component to app startup.

... something like that. I'll play with the wording on Monday morning and see what composes well.

In the meantime, I'm still waiting on Steve to get back to me. We'll see what he says, especially on my first question confirming that an import in this context is a dynamic import(). I'm almost certain that I'm correct on that point. That will confirm that I have the correct MDN cross-link and link text.

Have a great weekend! We'll push this forward on Monday.

@gregoryagu
Copy link

Have a great weekend too! (And if you ever want to hire a documentation reviewer/tester, let me know!)

@guardrex
Copy link
Collaborator Author

guardrex commented Jan 7, 2025

Here's where we're at with this. We'll only carry the remark warning devs off of placing a <script> tag. We won't get into adding a <script> tag with type="module" for latency at startup. If devs want to do that, there's nothing stopping them AFAIK. However, that's not a best/typical use case scenario for loading JS module code using the collocation approach. I'll take more feedback on it going forward. If a few more devs open issues asking about it in the coming months/years, then we probably will add something on that aspect.

Thanks for the issue! I'm glad we're improving the coverage to dispel any question about it. Happy New Year! 🎉

@guardrex guardrex merged commit 5c3c045 into main Jan 7, 2025
3 checks passed
@guardrex guardrex deleted the guardrex/blazor-js-interop-modules branch January 7, 2025 14:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants