-
Notifications
You must be signed in to change notification settings - Fork 26.3k
refactor(core): update FakeNavigation to the latest spec #62017
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
base: main
Are you sure you want to change the base?
Conversation
Spec updates are in whatwg/html#10919 For the most part, the updates revolve around the deferred commit handling (with precommitHandler). Updates to redirect allow more options. A committed promise now exists on the transition since commits can be delayed. Tests were made zoneless for easier debugging and timeouts were reduced.
45e41dd
to
63e7890
Compare
@@ -457,6 +458,17 @@ export class FakeNavigation implements Navigation { | |||
return this.disposed; | |||
} | |||
|
|||
abortTheOngoingNavigation(eventToAbort: InternalFakeNavigateEvent, reason?: Error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: abortOngoingNavigation
I never use the
in method names
@@ -457,6 +458,17 @@ export class FakeNavigation implements Navigation { | |||
return this.disposed; | |||
} | |||
|
|||
abortTheOngoingNavigation(eventToAbort: InternalFakeNavigateEvent, reason?: Error) { | |||
if (!this.navigateEvent || this.navigateEvent !== eventToAbort) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: you never have to check whether navigateEvent is truthy, since comparing undefined/null to eventToAbort
is always false.
Simplify to if (this.navigateEvent !== eventToAbort)
} | ||
const navigateerrorEvent = new Event('navigateerror', {bubbles: false, cancelable}); | ||
const navigateerrorEvent = new Event('navigateerror', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the ErrorEvent
constructor so you can pass error
in without having to cast.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don’t think it exists in node nodejs/node#47587. I was getting an error trying to use it
} | ||
result.finishedReject(reason); | ||
const transition = navigation.transition as InternalNavigationTransition; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as InternalNavigationTransition | undefined;
otherwise it looks like your ?
is needless
TESTED=TGP |
Spec updates are in whatwg/html#10919
For the most part, the updates revolve around the deferred commit handling (with precommitHandler). Updates to redirect allow more options. A committed promise now exists on the transition since commits can be delayed. Tests were made zoneless for easier debugging and timeouts were reduced.