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

Test inserted async script execution order #45660

Merged
merged 3 commits into from
Apr 15, 2024
Merged

Conversation

domenic
Copy link
Member

@domenic domenic commented Apr 11, 2024

  • Chromium passes these tests
  • WebKit almost passes, but suffers from https://bugs.webkit.org/show_bug.cgi?id=272505: for some reason it fires the error event from script 2 before executing script 1
  • Gecko fails these tests. It both executes script 1 and fires the error event from script 2 synchronously.

whatwg/html#9864 discusses the issue. whatwg/html#10272 matches what is tested here.

Copy link
Member

@foolip foolip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very nice and clear. I haven't reviewed the spec text to confirm this is the required behavior, as I assume that is not in question.

@domfarolino
Copy link
Member

Chrome and Safari seem to run them async but not using microtask timing

I am assuming Chrome & Safari both run them async in a single posted task (not microtask)? Could you maybe modify the tests locally to see if instead of using queueMicrotask() here, if you used setTimeout() (or maybe postTask()?) the tests pass in those browsers?

If they do, then maybe the easiest thing to do is just align the spec to both of those implementations to at least match reality.

Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you insert a script like this, is it essentially synchronous even though it says "async", except parsing/execution happens after a single microtask? What if they contain further import statements?

Because if we do actually go in parallel somewhere here I agree that we need a task.

@domenic
Copy link
Member Author

domenic commented Apr 12, 2024

I am assuming Chrome & Safari both run them async in a single posted task (not microtask)? Could you maybe modify the tests locally to see if instead of using queueMicrotask() here, if you used setTimeout() (or maybe postTask()?) the tests pass in those browsers?

When doing setTimeout(,0), it becomes a race condition, presumably because whatever task source they're using for scheduling the script execution (networking task source?) is not always faster than the timer task source. setTimeout(,10) works.

When you insert a script like this, is it essentially synchronous even though it says "async", except parsing/execution happens after a single microtask?

This question made me re-inspect the spec. It seems I got it slightly wrong. Let's discuss over in whatwg/html#9864.

@domenic domenic merged commit 5adce6f into master Apr 15, 2024
20 checks passed
@domenic domenic deleted the module-script-sync-error branch April 15, 2024 04:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants