Skip to content

Commit a3bd65e

Browse files
atscottatscott
committed
fix(router): Ensure APP_INITIALIZER of enabledBlocking option completes (#46634)
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 #46634
1 parent 88298ca commit a3bd65e

File tree

5 files changed

+95
-45
lines changed

5 files changed

+95
-45
lines changed

‎packages/router/src/apply_redirects.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {catchError, concatMap, first, last, map, mergeMap, scan, tap} from 'rxjs
1313
import {CanLoadFn, LoadedRouterConfig, Route, Routes} from './models';
1414
import {prioritizedGuardValue} from './operators/prioritized_guard_value';
1515
import {RouterConfigLoader} from './router_config_loader';
16-
import {navigationCancelingError, Params, PRIMARY_OUTLET} from './shared';
16+
import {navigationCancelingError, Params, PRIMARY_OUTLET, REDIRECTING_CANCELLATION_REASON} from './shared';
1717
import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
1818
import {forEach, wrapIntoObservable} from './utils/collection';
1919
import {getOrCreateRouteInjectorIfNeeded, getOutlet, sortByMatchingOutlets} from './utils/config';
@@ -378,7 +378,7 @@ class ApplyRedirects {
378378
if (!isUrlTree(result)) return;
379379

380380
const error: Error&{url?: UrlTree} = navigationCancelingError(
381-
`Redirecting to "${this.urlSerializer.serialize(result)}"`);
381+
REDIRECTING_CANCELLATION_REASON + this.urlSerializer.serialize(result));
382382
error.url = result;
383383
throw error;
384384
}),

‎packages/router/src/router.ts‎

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

412412
private locationSubscription?: SubscriptionLike;
413-
private navigationId: number = 0;
413+
/** @internal */
414+
navigationId: number = 0;
414415

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

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

849+
switchTap(() => this.afterPreactivation()),
850+
849851
map((t: NavigationTransition) => {
850852
const targetRouterState = createRouterState(
851853
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,12 +9,13 @@
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} 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';
17-
import {Event, stringifyEvent} from './events';
18+
import {Event, NavigationCancel, NavigationEnd, NavigationError, stringifyEvent} from './events';
1819
import {Route, Routes} from './models';
1920
import {DefaultTitleStrategy, TitleStrategy} from './page_title_strategy';
2021
import {RouteReuseStrategy} from './route_reuse_strategy';
@@ -24,6 +25,7 @@ import {ChildrenOutletContexts} from './router_outlet_context';
2425
import {PreloadingStrategy, RouterPreloader} from './router_preloader';
2526
import {ROUTER_SCROLLER, RouterScroller} from './router_scroller';
2627
import {ActivatedRoute} from './router_state';
28+
import {REDIRECTING_CANCELLATION_REASON} from './shared';
2729
import {UrlHandlingStrategy} from './url_handling_strategy';
2830
import {DefaultUrlSerializer, UrlSerializer, UrlTree} from './url_tree';
2931
import {flatten} from './utils/collection';
@@ -588,18 +590,63 @@ function provideEnabledBlockingInitialNavigation(): Provider {
588590
injector.get(LOCATION_INITIALIZED, Promise.resolve(null));
589591
let initNavigation = false;
590592

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

597642
router.afterPreactivation = () => {
598-
// only the initial navigation should be delayed
643+
// Unblock APP_INITIALIZER once we get to `afterPreactivation`. At this point, we
644+
// assume activation will complete successfully (even though this is not
645+
// guaranteed).
646+
resolve(true);
647+
// only the initial navigation should be delayed until bootstrapping is done.
599648
if (!initNavigation) {
600-
initNavigation = true;
601-
resolve(true);
602-
return bootstrapDone;
649+
return bootstrapDone.closed ? of(void 0) : bootstrapDone;
603650
// subsequent navigations should not be delayed
604651
} else {
605652
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) {

‎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)