-
Notifications
You must be signed in to change notification settings - Fork 25.3k
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
base: main
Are you sure you want to change the base?
Conversation
@gregoryagu ... I think I'll need to do a little more work ...
|
I think on the second point that it's best to remove the remark. I really think that you can probably add a 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. |
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 use 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: However, as you point out, if you add type="module" then it is only loaded the first time.
This is the Repository |
Here is how I would write it: Don't place a |
Thanks for the additional investigation.
Good! My hunch was correct 😄, and that moves the latency to app startup, which most people probably wouldn't want.
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 ...
... 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 Have a great weekend! We'll push this forward on Monday. |
Have a great weekend too! (And if you ever want to hire a documentation reviewer/tester, let me know!) |
Fixes #34414
Thanks @gregoryagu! 🚀 ... I won't merge this until tomorrow morning (Friday) to give you a chance to review it.
Internal previews