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

Should module scripts always fire load events? #6421

Open
jakearchibald opened this issue Feb 26, 2021 · 10 comments
Open

Should module scripts always fire load events? #6421

jakearchibald opened this issue Feb 26, 2021 · 10 comments
Labels
addition/proposal New features or enhancements topic: script

Comments

@jakearchibald
Copy link
Contributor

const script = document.createElement('script');
script.textContent = source;
script.type = 'module';
document.head.append(script);

How would I know when the above script has executed?

  • We know the answer with classic/module scripts with a src, as there's a "load" event.
  • We know the answer with classic scripts without a src, as it's synchronous.

However, module scripts without a src still execute asynchronously, but there's no "load" event.

It feels like we should dispatch a "load" event for all module scripts, not just those from an external file. WDYT @domenic?

This was raised in https://bugs.chromium.org/p/chromium/issues/detail?id=1180532#c8.

@annevk annevk added the topic: agent The interaction with JavaScript's agent and agent cluster concepts label Feb 26, 2021
@domenic domenic added addition/proposal New features or enhancements topic: script and removed topic: agent The interaction with JavaScript's agent and agent cluster concepts labels Feb 26, 2021
@domenic
Copy link
Member

domenic commented Feb 26, 2021

This makes sense to me. It sounds like from the bug you linked browsers are not interoperable here. @hiroshige-g do you have any interest in writing web platform tests for this case as the next step?

@annevk
Copy link
Member

annevk commented Mar 1, 2021

cc @codehag

@rniwa
Copy link

rniwa commented Mar 2, 2021

I guess this has never been an issue with classic script because inline classic script can be deferred?

@rniwa
Copy link

rniwa commented Mar 2, 2021

Can't you just check document.readyState and/or wait for DOMContentLoaded since deferred scripts all run at that time? I guess I'd like see the concrete use cases for this.

An event might be the right API. Or maybe we just need to expose a boolean indicating whether the script has already executed or not. If the use case involves doing something in the case the script hadn't run yet or never runs, it's not useful to have a load event since load event will never fire in that case so you'd have to check the above condition anyway.

@jakearchibald
Copy link
Contributor Author

Can't you just check document.readyState and/or wait for DOMContentLoaded since deferred scripts all run at that time?

Only parsed scripts can end up in the deferred queue, appended scripts like above are just async. Besides, an inline module script could run much later than DOMContentLoaded due to import fetches and top-level awaits.

Or maybe we just need to expose a boolean indicating whether the script has already executed or not. If the use case involves doing something in the case the script hadn't run yet or never runs, it's not useful to have a load event since load event will never fire in that case so you'd have to check the above condition anyway.

I think we should use a "load" event, since that's what we already use for other scripts that execute on some async delay.

We should be putting promises on everything with "load" & "error" events too, but that's for another issue.

@annevk
Copy link
Member

annevk commented Mar 2, 2021

(See #127 for the ready promise, though maybe it's easier for things that fetch things.)

@rniwa
Copy link

rniwa commented Mar 3, 2021

Can't you just check document.readyState and/or wait for DOMContentLoaded since deferred scripts all run at that time?

Only parsed scripts can end up in the deferred queue, appended scripts like above are just async. Besides, an inline module script could run much later than DOMContentLoaded due to import fetches and top-level awaits.

What are use cases / concrete scenarios in which checking whether such a script has ran is useful?

Or maybe we just need to expose a boolean indicating whether the script has already executed or not. If the use case involves doing something in the case the script hadn't run yet or never runs, it's not useful to have a load event since load event will never fire in that case so you'd have to check the above condition anyway.

I think we should use a "load" event, since that's what we already use for other scripts that execute on some async delay.

I don't find that argument compelling without knowing other places in the platform where we'd fire load event on things that don't involve resource loading.

@annevk
Copy link
Member

annevk commented Mar 3, 2021

(SVG and initial about:blank in <iframe> come to mind. Neither great precedent to cite though.)

@jakearchibald
Copy link
Contributor Author

jakearchibald commented Mar 3, 2021

@rniwa

What are use cases / concrete scenarios in which checking whether such a script has ran is useful?

In the Chromium report, it seems like the script is being slowly ported from classic to module. The developer is expecting particular global state & side effects to have been applied once the script has ran.

There are other more 'ideal' ways to do this in a purely module world, but that isn't always how it goes in the real world 😄

I don't find that argument compelling without knowing other places in the platform where we'd fire load event on things that don't involve resource loading.

But that's the point, it might involve resource loading. An inline module script can include imports, and top-level await.

const script = document.createElement('script');
script.textContent = `
  import { someURL } from './foo';
  const response = await fetch(someURL);
`;
script.type = 'module';
document.head.append(script);

We could consider other types of resource that have "load" events, but it feels more important to be consistent within <script>.

Inline classic script Inline module script Classic script with src Module script with src
Fires "load" event once executed
Loads async when appended with JS
Fetches script ✅ *

* You could say an inline module without imports (or equivalent top-level-awaits) doesn't fetch script, but do we really want to toggle behaviour on the script's source like that?

It seems like, when the spec was written, the author thought they were being consistent by making inline module scripts behave like inline classic scripts in terms of the "load" event. However, in reality, inline module scripts have more behaviour in common with scripts with src.

@rniwa
Copy link

rniwa commented Mar 3, 2021

Actually, style element would fire load event as well when all the critical sub-resources of inline stylesheet have been loaded so it seems like firing load event here seems fine:
https://html.spec.whatwg.org/multipage/semantics.html#the-style-element

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements topic: script
Development

No branches or pull requests

4 participants