-
Notifications
You must be signed in to change notification settings - Fork 27.3k
[4.x only] fix(router): do not finish bootstrap until all the routes are resolved #14155
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -278,6 +278,18 @@ type NavigationParams = { | |
| source: NavigationSource, | ||
| }; | ||
|
|
||
| /** | ||
| * @internal | ||
| */ | ||
| export type RouterHook = (snapshot: RouterStateSnapshot) => Observable<void>; | ||
|
|
||
| /** | ||
| * @internal | ||
| */ | ||
| function defaultRouterHook(snapshot: RouterStateSnapshot): Observable<void> { | ||
| return of (null); | ||
| } | ||
|
|
||
|
|
||
| /** | ||
| * Does not detach any subtrees. Reuses routes as long as their route config is the same. | ||
|
|
@@ -320,11 +332,23 @@ export class Router { | |
| */ | ||
| errorHandler: ErrorHandler = defaultErrorHandler; | ||
|
|
||
|
|
||
|
|
||
| /** | ||
| * Indicates if at least one navigation happened. | ||
| */ | ||
| navigated: boolean = false; | ||
|
|
||
| /** | ||
| * Used by RouterModule. This allows us to | ||
| * pause the navigation either before preactivation or after it. | ||
| * @internal | ||
| */ | ||
| hooks: {beforePreactivation: RouterHook, afterPreactivation: RouterHook} = { | ||
| beforePreactivation: defaultRouterHook, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should the default be null ? (what is the benefit of having an explicit default ?)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. so we don't have to check for null and use polymorphism to handle this case. We use the same approach to handle other defaults in the router (e.g., defaultErrorHandler) |
||
| afterPreactivation: defaultRouterHook | ||
| }; | ||
|
|
||
| /** | ||
| * Extracts and merges URLs. Used for AngularJS to Angular migrations. | ||
| */ | ||
|
|
@@ -679,26 +703,33 @@ export class Router { | |
| urlAndSnapshot$ = of ({appliedUrl: url, snapshot: precreatedState}); | ||
| } | ||
|
|
||
| const beforePreactivationDone$ = mergeMap.call( | ||
| urlAndSnapshot$, (p: {appliedUrl: string, snapshot: RouterStateSnapshot}) => { | ||
| return map.call(this.hooks.beforePreactivation(p.snapshot), () => p); | ||
| }); | ||
|
|
||
| // run preactivation: guards and data resolvers | ||
| let preActivation: PreActivation; | ||
| const preactivationTraverse$ = map.call(urlAndSnapshot$, ({appliedUrl, snapshot}: any) => { | ||
| preActivation = | ||
| new PreActivation(snapshot, this.currentRouterState.snapshot, this.injector); | ||
| preActivation.traverse(this.outletMap); | ||
| return {appliedUrl, snapshot}; | ||
| }); | ||
| const preactivationTraverse$ = map.call( | ||
| beforePreactivationDone$, | ||
| ({appliedUrl, snapshot}: {appliedUrl: string, snapshot: RouterStateSnapshot}) => { | ||
| preActivation = | ||
| new PreActivation(snapshot, this.currentRouterState.snapshot, this.injector); | ||
| preActivation.traverse(this.outletMap); | ||
| return {appliedUrl, snapshot}; | ||
| }); | ||
|
|
||
| const preactivationCheckGuards = | ||
| mergeMap.call(preactivationTraverse$, ({appliedUrl, snapshot}: any) => { | ||
| const preactivationCheckGuards$ = mergeMap.call( | ||
| preactivationTraverse$, | ||
| ({appliedUrl, snapshot}: {appliedUrl: string, snapshot: RouterStateSnapshot}) => { | ||
| if (this.navigationId !== id) return of (false); | ||
|
|
||
| return map.call(preActivation.checkGuards(), (shouldActivate: boolean) => { | ||
| return {appliedUrl: appliedUrl, snapshot: snapshot, shouldActivate: shouldActivate}; | ||
| }); | ||
| }); | ||
|
|
||
| const preactivationResolveData$ = mergeMap.call(preactivationCheckGuards, (p: any) => { | ||
| const preactivationResolveData$ = mergeMap.call(preactivationCheckGuards$, (p: any) => { | ||
| if (this.navigationId !== id) return of (false); | ||
|
|
||
| if (p.shouldActivate) { | ||
|
|
@@ -708,11 +739,15 @@ export class Router { | |
| } | ||
| }); | ||
|
|
||
| const preactivationDone$ = mergeMap.call(preactivationResolveData$, (p: any) => { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. any -> type
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ? |
||
| return map.call(this.hooks.afterPreactivation(p.snapshot), () => p); | ||
| }); | ||
|
|
||
|
|
||
| // create router state | ||
| // this operation has side effects => route state is being affected | ||
| const routerState$ = | ||
| map.call(preactivationResolveData$, ({appliedUrl, snapshot, shouldActivate}: any) => { | ||
| map.call(preactivationDone$, ({appliedUrl, snapshot, shouldActivate}: any) => { | ||
| if (shouldActivate) { | ||
| const state = | ||
| createRouterState(this.routeReuseStrategy, snapshot, this.currentRouterState); | ||
|
|
||
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.
void vs null ?
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.
voidisn't an expression. I can useundefined, but I don't think we use undefined very much in the codebase, that's why I chose null.