Skip to content

Commit e8c7dd1

Browse files
atscottatscott
committed
fix(router): Ensure APP_INITIALIZER of enabledBlocking option completes (#46026)
Previously, if `initialNavigation` were set to `enabledBlocking`, the Router's `APP_INITIALIZER` would never resolve if that initial navigation failed. This results in the application load hanging and never completing. fixes #44355 PR Close #46026
1 parent 598239c commit e8c7dd1

5 files changed

Lines changed: 96 additions & 46 deletions

File tree

‎packages/router/src/operators/check_guards.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import {ActivationStart, ChildActivationStart, Event} from '../events';
1515
import {CanActivateChildFn, CanActivateFn, CanDeactivateFn, CanLoadFn, CanMatch, CanMatchFn, Route} from '../models';
1616
import {NavigationTransition} from '../router';
1717
import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state';
18-
import {navigationCancelingError} from '../shared';
18+
import {navigationCancelingError, REDIRECTING_CANCELLATION_REASON} from '../shared';
1919
import {UrlSegment, UrlSerializer, UrlTree} from '../url_tree';
2020
import {wrapIntoObservable} from '../utils/collection';
2121
import {CanActivate, CanDeactivate, getCanActivateChild, getToken} from '../utils/preactivation';
@@ -219,8 +219,8 @@ function redirectIfUrlTree(urlSerializer: UrlSerializer):
219219
tap((result: UrlTree|boolean) => {
220220
if (!isUrlTree(result)) return;
221221

222-
const error: Error&{url?: UrlTree} =
223-
navigationCancelingError(`Redirecting to "${urlSerializer.serialize(result)}"`);
222+
const error: Error&{url?: UrlTree} = navigationCancelingError(
223+
REDIRECTING_CANCELLATION_REASON + urlSerializer.serialize(result));
224224
error.url = result;
225225
throw error;
226226
}),

‎packages/router/src/router.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {DefaultRouteReuseStrategy, RouteReuseStrategy} from './route_reuse_strat
2727
import {RouterConfigLoader} from './router_config_loader';
2828
import {ChildrenOutletContexts} from './router_outlet_context';
2929
import {ActivatedRoute, ActivatedRouteSnapshot, createEmptyState, RouterState, RouterStateSnapshot} from './router_state';
30-
import {isNavigationCancelingError, navigationCancelingError, Params} from './shared';
30+
import {isNavigationCancelingError, navigationCancelingError, Params, REDIRECTING_CANCELLATION_REASON} from './shared';
3131
import {DefaultUrlHandlingStrategy, UrlHandlingStrategy} from './url_handling_strategy';
3232
import {containsTree, createEmptyUrlTree, IsActiveMatchOptions, UrlSerializer, UrlTree} from './url_tree';
3333
import {standardizeConfig, validateConfig} from './utils/config';
@@ -411,7 +411,8 @@ export class Router {
411411
private disposed = false;
412412

413413
private locationSubscription?: SubscriptionLike;
414-
private navigationId: number = 0;
414+
/** @internal */
415+
navigationId: number = 0;
415416

416417
/**
417418
* The id of the currently active page in the router.
@@ -762,7 +763,8 @@ export class Router {
762763
tap(t => {
763764
if (isUrlTree(t.guardsResult)) {
764765
const error: Error&{url?: UrlTree} = navigationCancelingError(
765-
`Redirecting to "${this.serializeUrl(t.guardsResult)}"`);
766+
REDIRECTING_CANCELLATION_REASON +
767+
`"${this.serializeUrl(t.guardsResult)}"`);
766768
error.url = t.guardsResult;
767769
throw error;
768770
}
@@ -821,8 +823,6 @@ export class Router {
821823
return undefined;
822824
}),
823825

824-
switchTap(() => this.afterPreactivation()),
825-
826826
// --- LOAD COMPONENTS ---
827827
switchTap((t: NavigationTransition) => {
828828
const loadComponents =
@@ -847,6 +847,8 @@ export class Router {
847847
.pipe(defaultIfEmpty(), take(1));
848848
}),
849849

850+
switchTap(() => this.afterPreactivation()),
851+
850852
map((t: NavigationTransition) => {
851853
const targetRouterState = createRouterState(
852854
this.routeReuseStrategy, t.targetSnapshot!, t.currentRouterState);

‎packages/router/src/router_module.ts‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@
99
import {HashLocationStrategy, Location, LOCATION_INITIALIZED, LocationStrategy, PathLocationStrategy, ViewportScroller} from '@angular/common';
1010
import {ANALYZE_FOR_ENTRY_COMPONENTS, APP_BOOTSTRAP_LISTENER, APP_INITIALIZER, ApplicationRef, Compiler, ComponentRef, ENVIRONMENT_INITIALIZER, Inject, inject, InjectFlags, InjectionToken, Injector, ModuleWithProviders, NgModule, NgProbeToken, Optional, Provider, SkipSelf, Type, ɵRuntimeError as RuntimeError} from '@angular/core';
1111
import {of, Subject} from 'rxjs';
12+
import {filter, map, take} from 'rxjs/operators';
1213

1314
import {EmptyOutletComponent} from './components/empty_outlet';
1415
import {RouterLink, RouterLinkWithHref} from './directives/router_link';
1516
import {RouterLinkActive} from './directives/router_link_active';
1617
import {RouterOutlet} from './directives/router_outlet';
1718
import {RuntimeErrorCode} from './errors';
18-
import {Event, stringifyEvent} from './events';
19+
import {Event, NavigationCancel, NavigationEnd, NavigationError, stringifyEvent} from './events';
1920
import {Route, Routes} from './models';
2021
import {DefaultTitleStrategy, TitleStrategy} from './page_title_strategy';
2122
import {RouteReuseStrategy} from './route_reuse_strategy';
@@ -25,6 +26,7 @@ import {ChildrenOutletContexts} from './router_outlet_context';
2526
import {PreloadingStrategy, RouterPreloader} from './router_preloader';
2627
import {ROUTER_SCROLLER, RouterScroller} from './router_scroller';
2728
import {ActivatedRoute} from './router_state';
29+
import {REDIRECTING_CANCELLATION_REASON} from './shared';
2830
import {UrlHandlingStrategy} from './url_handling_strategy';
2931
import {DefaultUrlSerializer, UrlSerializer, UrlTree} from './url_tree';
3032
import {flatten} from './utils/collection';
@@ -590,18 +592,63 @@ function provideEnabledBlockingInitialNavigation(): Provider {
590592
injector.get(LOCATION_INITIALIZED, Promise.resolve(null));
591593
let initNavigation = false;
592594

595+
/**
596+
* Performs the given action once the router finishes its next/current navigation.
597+
*
598+
* If the navigation is canceled or errors without a redirect, the navigation is considered
599+
* complete. If the `NavigationEnd` event emits, the navigation is also considered complete.
600+
*/
601+
function afterNextNavigation(action: () => void) {
602+
const router = injector.get(Router);
603+
router.events
604+
.pipe(
605+
filter(
606+
(e): e is NavigationEnd|NavigationCancel|NavigationError =>
607+
e instanceof NavigationEnd || e instanceof NavigationCancel ||
608+
e instanceof NavigationError),
609+
map(e => {
610+
if (e instanceof NavigationEnd) {
611+
// Navigation assumed to succeed if we get `ActivationStart`
612+
return true;
613+
}
614+
const newNavigationStarted = router.navigationId !== e.id;
615+
// TODO(atscott): Do not rely on the string reason to determine if cancelation
616+
// is redirecting
617+
const redirectingWithUrlTree = e instanceof NavigationCancel ?
618+
e.reason.indexOf(REDIRECTING_CANCELLATION_REASON) !== -1 :
619+
false;
620+
// Navigation failed, but if we already have a new navigation, wait for the
621+
// result of that one instead.
622+
return newNavigationStarted || redirectingWithUrlTree ? null : false;
623+
}),
624+
filter((result): result is boolean => result !== null),
625+
take(1),
626+
)
627+
.subscribe(() => {
628+
action();
629+
});
630+
}
631+
593632
return () => {
594633
return locationInitialized.then(() => {
595634
return new Promise(resolve => {
596635
const router = injector.get(Router);
597636
const bootstrapDone = injector.get(BOOTSTRAP_DONE);
637+
afterNextNavigation(() => {
638+
// Unblock APP_INITIALIZER in case the initial navigation was canceled or errored
639+
// without a redirect.
640+
resolve(true);
641+
initNavigation = true;
642+
});
598643

599644
router.afterPreactivation = () => {
600-
// only the initial navigation should be delayed
645+
// Unblock APP_INITIALIZER once we get to `afterPreactivation`. At this point, we
646+
// assume activation will complete successfully (even though this is not
647+
// guaranteed).
648+
resolve(true);
649+
// only the initial navigation should be delayed until bootstrapping is done.
601650
if (!initNavigation) {
602-
initNavigation = true;
603-
resolve(true);
604-
return bootstrapDone;
651+
return bootstrapDone.closed ? of(void 0) : bootstrapDone;
605652
// subsequent navigations should not be delayed
606653
} else {
607654
return of(void 0);

‎packages/router/src/shared.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,7 @@ export function convertToParamMap(params: Params): ParamMap {
111111
return new ParamsAsMap(params);
112112
}
113113

114+
export const REDIRECTING_CANCELLATION_REASON = 'Redirecting to ';
114115
const NAVIGATION_CANCELING_ERROR = 'ngNavigationCancelingError';
115116

116117
export function navigationCancelingError(message: string|null|false) {

‎packages/router/test/bootstrap.spec.ts‎

Lines changed: 33 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -77,40 +77,40 @@ describe('bootstrap', () => {
7777
}
7878
}));
7979

80-
// TODO(#44355): bootstrapping fails when the initial navigation fails
81-
xit('should complete resolvers when initial navigation fails and initialNavigation = enabledBlocking',
82-
async () => {
83-
@NgModule({
84-
imports: [
85-
BrowserModule,
86-
RouterModule.forRoot(
87-
[{
88-
matcher: () => {
89-
throw new Error('error in matcher');
90-
},
91-
children: []
92-
}],
93-
{useHash: true, initialNavigation: 'enabledBlocking'})
94-
],
95-
declarations: [RootCmp],
96-
bootstrap: [RootCmp],
97-
providers: [...testProviders],
98-
schemas: [CUSTOM_ELEMENTS_SCHEMA]
99-
})
100-
class TestModule {
101-
constructor() {
102-
log.push('TestModule');
103-
}
104-
}
80+
it('should complete resolvers when initial navigation fails and initialNavigation = enabledBlocking',
81+
async () => {
82+
@NgModule({
83+
imports: [
84+
BrowserModule,
85+
RouterModule.forRoot(
86+
[{
87+
matcher: () => {
88+
throw new Error('error in matcher');
89+
},
90+
children: []
91+
}],
92+
{useHash: true, initialNavigation: 'enabledBlocking'})
93+
],
94+
declarations: [RootCmp],
95+
bootstrap: [RootCmp],
96+
providers: [...testProviders],
97+
schemas: [CUSTOM_ELEMENTS_SCHEMA]
98+
})
99+
class TestModule {
100+
constructor(router: Router) {
101+
log.push('TestModule');
102+
router.events.subscribe(e => log.push(e.constructor.name));
103+
}
104+
}
105105

106-
await platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => {
107-
const router = res.injector.get(Router);
108-
expect(router.navigated).toEqual(false);
109-
expect(router.getCurrentNavigation()).toBeNull();
110-
expect(log).toContain('TestModule');
111-
expect(log).toContain('NavigationError');
112-
});
113-
});
106+
await platformBrowserDynamic([]).bootstrapModule(TestModule).then(res => {
107+
const router = res.injector.get(Router);
108+
expect(router.navigated).toEqual(false);
109+
expect(router.getCurrentNavigation()).toBeNull();
110+
expect(log).toContain('TestModule');
111+
expect(log).toContain('NavigationError');
112+
});
113+
});
114114

115115
it('should wait for redirect when initialNavigation = enabledBlocking', async () => {
116116
@Injectable({providedIn: 'root'})

0 commit comments

Comments
 (0)