Skip to content

Fix various event and promise issues, especially timing #200

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

Merged
merged 2 commits into from
Feb 4, 2022

Conversation

domenic
Copy link
Collaborator

@domenic domenic commented Jan 26, 2022

Specifically:

  • Mark finished promises as handled.

  • Change when currentchange fires slightly, so that currentchange handlers can't cause microtasks to run at an unusual time and thus swap the usual ordering of events and promises. This also allows us to clean up the "did finish before commit" bit since, contrary to the note in the spec, it was actually only necessary because of the currentchange-triggers-microtasks problem.

  • Always "wait for all" on at least one promise, since the zero-promise special case causes timing changes.

Closes #199.

This corresponds to the Chromium change in https://chromium-review.googlesource.com/c/chromium/src/+/3413934.


This currently depends on whatwg/webidl#1090 which will hopefully be merged soon; the build won't pass without that.


Preview | Diff

Specifically:

* Mark finished promises as handled.

* Change when currentchange fires slightly, so that currentchange handlers can't cause microtasks to run at an unusual time and thus swap the usual ordering of events and promises. This also allows us to clean up the "did finish before commit" bit since, contrary to the note in the spec, it was actually only necessary because of the currentchange-triggers-microtasks problem.

* Always "wait for all" on at least one promise, since the zero-promise special case causes timing changes.

Closes #199.

This corresponds to the Chromium change in https://chromium-review.googlesource.com/c/chromium/src/+/3405377.
@domenic domenic force-pushed the fix-ordering-issues branch from 176d62c to ce8cc00 Compare January 26, 2022 22:56
@domenic domenic merged commit 2a730b7 into main Feb 4, 2022
@domenic domenic deleted the fix-ordering-issues branch February 4, 2022 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

appHistory.transition.finished/navigatesuccess/navigateerror can be fired before finished promise
1 participant