whatwg / html

HTML Standard
https://html.spec.whatwg.org/multipage/
Other
8.03k stars 2.62k forks source link

Should module scripts always fire load events? #6421

Open jakearchibald opened 3 years ago

jakearchibald commented 3 years ago
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?

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.

domenic commented 3 years ago

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 commented 3 years ago

cc @codehag

rniwa commented 3 years ago

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

rniwa commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

rniwa commented 3 years ago

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 commented 3 years ago

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

jakearchibald commented 3 years ago

@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 commented 3 years ago

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