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

Confusing "async" completion of script fetching #3746

Closed
domenic opened this issue Jun 7, 2018 · 5 comments · Fixed by web-platform-tests/wpt#41081
Closed

Confusing "async" completion of script fetching #3746

domenic opened this issue Jun 7, 2018 · 5 comments · Fixed by web-platform-tests/wpt#41081

Comments

@domenic
Copy link
Member

domenic commented Jun 7, 2018

Pointed out by @jonco3.

The script fetching algorithms all use an "asynchronous completion" framework, which is not very well-defined. See whatwg/infra#181.

In particular, several people assume that "async" means "queue a task". But it's quite possible to not end up queuing a task, e.g. if you "fetch the descendants of and instantiate" a module with no descendants. So then the presence of the words "when this asynchronously completes" causes confusion.

The particular case we were concerned with was "prepare a script" step 25 for src-less module scripts, which says

Fetch the descendants of and instantiate script, given the destination "script". When this asynchronously completes, set the script's script to the result. At that time, the script is ready.

In code, the case in question is a classic inline script that inserts a module inline script:

<script>
console.log(1);

const s = document.createElement("script");
s.type = "module";
s.textContent = "console.log(2);";
document.body.appendChild(s);

console.log(3);
</script>

There are a couple of possible dimensions of consistency here:

  1. Have no-dependency module scripts be consistent with classic scripts, and have them execute immediately (logs 1 2 3).
  2. Have no-dependency module scripts be consistent with dependency-having module scripts, and have them execute from a queued task (logs 1 3 2).

Currently the spec picks (1), but in a confusing way because of the use of the word "asynchronously" which causes people to think maybe it's (2).

/cc @whatwg/modules. I'd love folks thoughts on whether (1) or (2) is more natural.

Side note: if we pick (2), then we should remove the "set currentScript to null" logic inside "execute a script block", since currentScript will always be null anyway.

Also, I might have gotten something wrong here, as this is subtle stuff; please feel free to check my logic.

@hiroshige-g
Copy link
Contributor

In the example, I expect the module script is evaluated in the "deferred" timing, and thus the result is (2), no matter whether fetch the descendants of and instantiate completes synchronously or asynchronously. This is the current behavior of Chromium.

I feel (2) is more consistent, in that module scripts are always asynchronous and non-parser-blocking.

@domenic
Copy link
Member Author

domenic commented Jun 7, 2018

Thanks. I might have gotten lost in the spec; would you be able to check my reasoning on which part of the spec calls "execute a script block" in this case?

My reading is that we reach the If the script's type is "module" case in step 26, which says

When the script is ready, execute the script block and then remove the element from the set of scripts that will execute as soon as possible.

and step 25 says

Fetch the descendants of and instantiate script, given the destination "script". When this asynchronously completes, set the script's script to the result. At that time, the script is ready.

So "the script is ready" immediately (synchronously).

Did I miss something?

@hiroshige-g
Copy link
Contributor

Or, sorry, you are correct. I missed the parser-inserted check in the first clause of Step 26.

Still, (2) is the current behavior of Chromium, because for module scripts Chromium considers the script is ready always asynchronously.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 25, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Mar 27, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
aarongable pushed a commit to chromium/chromium that referenced this issue Apr 5, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371802
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1126561}
@domfarolino
Copy link
Member

Just for completeness, I added a tentative WPT for this over in web-platform-tests/wpt#39205.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 5, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371802
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1126561}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Apr 5, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371802
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1126561}
cookiecrook pushed a commit to cookiecrook/wpt that referenced this issue Apr 8, 2023
Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371802
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1126561}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 15, 2023
…-import module script, a=testonly

Automatic update from web-platform-tests
Script: Add execution timing test for no-import module script

Both Chrome and Safari agree that module scripts with no imports run
asynchronously, per the semi-ambiguous language in the HTML Standard
discussed in whatwg/html#3746, despite the
script being "ready" immediately. Firefox seems to implement the spec
correctly, regarding the "readiness" being synchronous, however with
2/3 implementations *not* implementing the spec in this regard, and an
open spec issue in this area, I find it unlikely that Chromium's
will change its behavior (for compat). I'll recommend on the spec issue
that we change the spec to match Safari and Chrome, and land this test
asserting the majority behavior.

R=domenic@chromium.org

Bug: N/A
Change-Id: Ibeac277ab357b62e7d0a5e4afb5f95818a2d4005
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4371802
Commit-Queue: Dominic Farolino <dom@chromium.org>
Reviewed-by: Domenic Denicola <domenic@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1126561}

--

wpt-commits: f2151fce99621bd8e5526747f3a15d0df9741a38
wpt-pr: 39205
@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Jul 14, 2023

The spec now explicitly does (2), probably since #8253 that made the loading process consistently await the "loadingPromise" returned by ECMA-262's LoadRequestedModules() regardless of the number of dependencies.

domenic added a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2023
domenic added a commit to web-platform-tests/wpt that referenced this issue Jul 18, 2023
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 22, 2023
…ive, a=testonly

Automatic update from web-platform-tests
Mark non-external-noimport as non-tentative

See whatwg/html#3746 (comment). Closes whatwg/html#3746.

--

wpt-commits: 39ac6ad1cf7e29e07dae7c8ec487f2074bb0600d
wpt-pr: 41081
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Jul 24, 2023
…ive, a=testonly

Automatic update from web-platform-tests
Mark non-external-noimport as non-tentative

See whatwg/html#3746 (comment). Closes whatwg/html#3746.

--

wpt-commits: 39ac6ad1cf7e29e07dae7c8ec487f2074bb0600d
wpt-pr: 41081

UltraBlame original commit: a2c56c14c2d2bc69eca72b3f74ebcaa6f9d37ac4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Jul 24, 2023
…ive, a=testonly

Automatic update from web-platform-tests
Mark non-external-noimport as non-tentative

See whatwg/html#3746 (comment). Closes whatwg/html#3746.

--

wpt-commits: 39ac6ad1cf7e29e07dae7c8ec487f2074bb0600d
wpt-pr: 41081

UltraBlame original commit: a2c56c14c2d2bc69eca72b3f74ebcaa6f9d37ac4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Jul 24, 2023
…ive, a=testonly

Automatic update from web-platform-tests
Mark non-external-noimport as non-tentative

See whatwg/html#3746 (comment). Closes whatwg/html#3746.

--

wpt-commits: 39ac6ad1cf7e29e07dae7c8ec487f2074bb0600d
wpt-pr: 41081

UltraBlame original commit: a2c56c14c2d2bc69eca72b3f74ebcaa6f9d37ac4
ErichDonGubler pushed a commit to erichdongubler-mozilla/firefox that referenced this issue Jul 24, 2023
…ive, a=testonly

Automatic update from web-platform-tests
Mark non-external-noimport as non-tentative

See whatwg/html#3746 (comment). Closes whatwg/html#3746.

--

wpt-commits: 39ac6ad1cf7e29e07dae7c8ec487f2074bb0600d
wpt-pr: 41081
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

4 participants