refactor(common): Update FakeNavigation deferred commit to use precom…#60652
refactor(common): Update FakeNavigation deferred commit to use precom…#60652atscott wants to merge 2 commits intoangular:mainfrom
Conversation
…mitHandler This commit updates the FakeNavigation implementation to match the spec's new `precommitHandler` which replaces the old `commit: 'after-transition'`.
packages/core/primitives/dom-navigation/testing/fake_navigation.ts
Outdated
Show resolved
Hide resolved
packages/core/primitives/dom-navigation/testing/fake_navigation.ts
Outdated
Show resolved
Hide resolved
| () => { | ||
| const promisesList = handlers.map((handler) => handler()); | ||
| if (promisesList.length === 0) { | ||
| promisesList.push(Promise.resolve()); |
There was a problem hiding this comment.
Promise.all([]) doesn't work for some reason? This is odd
There was a problem hiding this comment.
This follows the spec word for word. There is a slight difference in timing between Promise.all([]) and Promise.all([Promise.resolve()]).
both of these resolve with '1'
Promise.race([
Promise.all([]).then(() => '1'),
Promise.all([Promise.resolve()]).then('2')
])
Promise.race([
Promise.all([Promise.resolve()]).then('2'),
Promise.all([]).then(() => '1'),
])
- Let promisesList be an empty list.
- For each handler of event's navigation handler list:
- If promisesList's size is 0, then set promisesList to « a promise resolved with undefined ».
There is a subtle timing difference between how waiting for all schedules its success and failure steps when given zero promises versus ≥1 promises. For most uses of waiting for all, this does not matter. However, with this API, there are so many events and promise handlers which could fire around the same time that the difference is pretty easily observable: it can cause the event/promise handler sequence to vary. (Some of the events and promises involved include: navigatesuccess / navigateerror, currententrychange, dispose, apiMethodTracker's promises, and the navigation.transition.finished promise.)
… precommitHandler
|
This PR was merged into the repository by commit 36b9236. The changes were merged into the following branches: main, 19.2.x |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
…mitHandler
This commit updates the FakeNavigation implementation to match the spec's new
precommitHandlerwhich replaces the oldcommit: 'after-transition'.