Skip to content

Commit 5b53535

Browse files
atscottmmalerba
authored andcommitted
fix(router): Update recognize stage to use internally async/await (angular#62994)
This is effectively a revert of angular@72e6a94. Debugging the recognize stage is considerably easier with async/await stacks compared to rxjs. This also improves maintainability and is a better 1:1 with server-side logic that has been implemented to match and can be more easily kept in sync. This also ensures that the recognize step is always async, whereas it can sometimes be synchronous with rxjs. BREAKING CHANGE: Router navigations may take several additional microtasks to complete. Tests have been found to often be highly dependent on the exact timing of navigation completions with respect to the microtask queue. The most common fix for tests is to ensure all navigations have been completed before making assertions. On rare occasions, this can also affect production applications. This can be caused by multiple subscriptions to router state throughout the application, both of which trigger navigations that happened to not conflict with the previous timing. PR Close angular#62994
1 parent 78b2279 commit 5b53535

15 files changed

Lines changed: 809 additions & 838 deletions

packages/core/test/bundling/router/bundle.golden_symbols.json

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,7 @@
395395
"_stripIndexHtml",
396396
"_stripOrigin",
397397
"_wasLastNodeCreated",
398+
"abortSignalToObservable",
398399
"activateRoutes",
399400
"activeConsumer",
400401
"addAfterRenderSequencesForView",
@@ -865,7 +866,6 @@
865866
"joinWithSlash",
866867
"last",
867868
"last2",
868-
"last3",
869869
"lastNodeWasCreated",
870870
"lastSelectedElementIdx",
871871
"leaveDI",
@@ -930,7 +930,6 @@
930930
"ngZoneInstanceId",
931931
"noLeftoversInUrl",
932932
"noMatch",
933-
"noMatch",
934933
"noSideEffects",
935934
"nodeChildrenAsMap",
936935
"noop",
@@ -1028,8 +1027,6 @@
10281027
"saveContentQueryAndDirectiveIndex",
10291028
"saveNameToExportMap",
10301029
"saveResolvedLocalsInData",
1031-
"scan",
1032-
"scanInternals",
10331030
"scheduleArray",
10341031
"scheduleAsyncIterable",
10351032
"scheduleCallbackWithMicrotask",

packages/platform-server/test/incremental_hydration_spec.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2585,6 +2585,7 @@ describe('platform-server partial hydration integration', () => {
25852585

25862586
const routeLink = doc.getElementById('route-link')!;
25872587
routeLink.click();
2588+
await appRef.whenStable();
25882589
await allPendingDynamicImports();
25892590
appRef.tick();
25902591

packages/router/src/apply_redirects.ts

Lines changed: 40 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -7,57 +7,48 @@
77
*/
88

99
import {Injector, runInInjectionContext, ɵRuntimeError as RuntimeError} from '@angular/core';
10-
import {Observable, of, throwError} from 'rxjs';
11-
import {map} from 'rxjs/operators';
1210

1311
import {RuntimeErrorCode} from './errors';
1412
import {NavigationCancellationCode} from './events';
15-
import {LoadedRouterConfig, RedirectFunction, Route} from './models';
13+
import {RedirectFunction, Route} from './models';
1614
import {navigationCancelingError} from './navigation_canceling_error';
1715
import {ActivatedRouteSnapshot} from './router_state';
1816
import {Params, PRIMARY_OUTLET} from './shared';
1917
import {UrlSegment, UrlSegmentGroup, UrlSerializer, UrlTree} from './url_tree';
2018
import {wrapIntoObservable} from './utils/collection';
2119

22-
export class NoMatch {
20+
export const NO_MATCH_ERROR_NAME = 'ɵNoMatch';
21+
export class NoMatch extends Error {
22+
override readonly name: string = NO_MATCH_ERROR_NAME;
2323
public segmentGroup: UrlSegmentGroup | null;
2424

2525
constructor(segmentGroup?: UrlSegmentGroup) {
26+
super();
2627
this.segmentGroup = segmentGroup || null;
2728
}
2829
}
2930

31+
export const ABSOLUTE_REDIRECT_ERROR_NAME = 'ɵAbsoluteRedirect';
3032
export class AbsoluteRedirect extends Error {
33+
override readonly name: string = ABSOLUTE_REDIRECT_ERROR_NAME;
3134
constructor(public urlTree: UrlTree) {
3235
super();
3336
}
3437
}
3538

36-
export function noMatch(segmentGroup: UrlSegmentGroup): Observable<any> {
37-
return throwError(new NoMatch(segmentGroup));
38-
}
39-
40-
export function absoluteRedirect(newTree: UrlTree): Observable<any> {
41-
return throwError(new AbsoluteRedirect(newTree));
42-
}
43-
44-
export function namedOutletsRedirect(redirectTo: string): Observable<any> {
45-
return throwError(
46-
new RuntimeError(
47-
RuntimeErrorCode.NAMED_OUTLET_REDIRECT,
48-
(typeof ngDevMode === 'undefined' || ngDevMode) &&
49-
`Only absolute redirects can have named outlets. redirectTo: '${redirectTo}'`,
50-
),
39+
export function namedOutletsRedirect(redirectTo: string): never {
40+
throw new RuntimeError(
41+
RuntimeErrorCode.NAMED_OUTLET_REDIRECT,
42+
(typeof ngDevMode === 'undefined' || ngDevMode) &&
43+
`Only absolute redirects can have named outlets. redirectTo: '${redirectTo}'`,
5144
);
5245
}
5346

54-
export function canLoadFails(route: Route): Observable<LoadedRouterConfig> {
55-
return throwError(
56-
navigationCancelingError(
57-
(typeof ngDevMode === 'undefined' || ngDevMode) &&
58-
`Cannot load children because the guard of the route "path: '${route.path}'" returned false`,
59-
NavigationCancellationCode.GuardRejected,
60-
),
47+
export function canLoadFails(route: Route): never {
48+
throw navigationCancelingError(
49+
(typeof ngDevMode === 'undefined' || ngDevMode) &&
50+
`Cannot load children because the guard of the route "path: '${route.path}'" returned false`,
51+
NavigationCancellationCode.GuardRejected,
6152
);
6253
}
6354

@@ -67,49 +58,46 @@ export class ApplyRedirects {
6758
private urlTree: UrlTree,
6859
) {}
6960

70-
lineralizeSegments(route: Route, urlTree: UrlTree): Observable<UrlSegment[]> {
61+
async lineralizeSegments(route: Route, urlTree: UrlTree): Promise<UrlSegment[]> {
7162
let res: UrlSegment[] = [];
7263
let c = urlTree.root;
7364
while (true) {
7465
res = res.concat(c.segments);
7566
if (c.numberOfChildren === 0) {
76-
return of(res);
67+
return res;
7768
}
7869

7970
if (c.numberOfChildren > 1 || !c.children[PRIMARY_OUTLET]) {
80-
return namedOutletsRedirect(`${route.redirectTo!}`);
71+
throw namedOutletsRedirect(`${route.redirectTo!}`);
8172
}
8273

8374
c = c.children[PRIMARY_OUTLET];
8475
}
8576
}
8677

87-
applyRedirectCommands(
78+
async applyRedirectCommands(
8879
segments: UrlSegment[],
8980
redirectTo: string | RedirectFunction,
9081
posParams: {[k: string]: UrlSegment},
9182
currentSnapshot: ActivatedRouteSnapshot,
9283
injector: Injector,
93-
): Observable<UrlTree> {
94-
return getRedirectResult(redirectTo, currentSnapshot, injector).pipe(
95-
map((redirect) => {
96-
if (redirect instanceof UrlTree) {
97-
throw new AbsoluteRedirect(redirect);
98-
}
99-
100-
const newTree = this.applyRedirectCreateUrlTree(
101-
redirect,
102-
this.urlSerializer.parse(redirect),
103-
segments,
104-
posParams,
105-
);
106-
107-
if (redirect[0] === '/') {
108-
throw new AbsoluteRedirect(newTree);
109-
}
110-
return newTree;
111-
}),
84+
): Promise<UrlTree> {
85+
const redirect = await getRedirectResult(redirectTo, currentSnapshot, injector);
86+
if (redirect instanceof UrlTree) {
87+
throw new AbsoluteRedirect(redirect);
88+
}
89+
90+
const newTree = this.applyRedirectCreateUrlTree(
91+
redirect,
92+
this.urlSerializer.parse(redirect),
93+
segments,
94+
posParams,
11295
);
96+
97+
if (redirect[0] === '/') {
98+
throw new AbsoluteRedirect(newTree);
99+
}
100+
return newTree;
113101
}
114102

115103
applyRedirectCreateUrlTree(
@@ -201,15 +189,15 @@ function getRedirectResult(
201189
redirectTo: string | RedirectFunction,
202190
currentSnapshot: ActivatedRouteSnapshot,
203191
injector: Injector,
204-
): Observable<string | UrlTree> {
192+
): Promise<string | UrlTree> {
205193
if (typeof redirectTo === 'string') {
206-
return of(redirectTo);
194+
return Promise.resolve(redirectTo);
207195
}
208196
const redirectToFn = redirectTo;
209197
const {queryParams, fragment, routeConfig, url, outlet, params, data, title} = currentSnapshot;
210198
return wrapIntoObservable(
211199
runInInjectionContext(injector, () =>
212200
redirectToFn({params, data, queryParams, fragment, routeConfig, url, outlet, title}),
213201
),
214-
);
202+
).toPromise() as Promise<string | UrlTree>;
215203
}

packages/router/src/navigation_transition.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ import {UrlSerializer, UrlTree} from './url_tree';
9090
import {Checks, getAllRouteGuards} from './utils/preactivation';
9191
import {CREATE_VIEW_TRANSITION} from './utils/view_transition';
9292
import {getClosestRouteInjector} from './utils/config';
93+
import {abortSignalToObservable} from './utils/abort_signal_to_observable';
9394

9495
/**
9596
* @description
@@ -539,6 +540,7 @@ export class NavigationTransitions {
539540
router.config,
540541
this.urlSerializer,
541542
this.paramsInheritanceStrategy,
543+
overallTransitionState.abortController.signal,
542544
),
543545

544546
// Update URL if in `eager` update mode
@@ -778,12 +780,7 @@ export class NavigationTransitions {
778780
take(1),
779781

780782
takeUntil(
781-
new Observable<void>((subscriber) => {
782-
const abortSignal = overallTransitionState.abortController.signal;
783-
const handler = () => subscriber.next();
784-
abortSignal.addEventListener('abort', handler);
785-
return () => abortSignal.removeEventListener('abort', handler);
786-
}).pipe(
783+
abortSignalToObservable(overallTransitionState.abortController.signal).pipe(
787784
// Ignore aborts if we are already completed, canceled, or are in the activation stage (we have targetRouterState)
788785
filter(() => !completedOrAborted && !overallTransitionState.targetRouterState),
789786
tap(() => {
@@ -831,6 +828,7 @@ export class NavigationTransitions {
831828
),
832829

833830
finalize(() => {
831+
overallTransitionState.abortController.abort();
834832
/* When the navigation stream finishes either through error or success,
835833
* we set the `completed` or `errored` flag. However, there are some
836834
* situations where we could get here without either of those being set.

packages/router/src/operators/check_guards.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ import {
5151
} from '../utils/type_guards';
5252

5353
import {prioritizedGuardValue} from './prioritized_guard_value';
54+
import {takeUntilAbort} from '../utils/abort_signal_to_observable';
5455

5556
export function checkGuards(
5657
injector: EnvironmentInjector,
@@ -244,6 +245,7 @@ export function runCanLoadGuards(
244245
route: Route,
245246
segments: UrlSegment[],
246247
urlSerializer: UrlSerializer,
248+
abortSignal: AbortSignal,
247249
): Observable<boolean> {
248250
const canLoad = route.canLoad;
249251
if (canLoad === undefined || canLoad.length === 0) {
@@ -255,7 +257,7 @@ export function runCanLoadGuards(
255257
const guardVal = isCanLoad(guard)
256258
? guard.canLoad(route, segments)
257259
: runInInjectionContext(injector, () => (guard as CanLoadFn)(route, segments));
258-
return wrapIntoObservable(guardVal);
260+
return wrapIntoObservable(guardVal).pipe(takeUntilAbort(abortSignal));
259261
});
260262

261263
return of(canLoadObservables).pipe(prioritizedGuardValue(), redirectIfUrlTree(urlSerializer));
@@ -277,6 +279,7 @@ export function runCanMatchGuards(
277279
route: Route,
278280
segments: UrlSegment[],
279281
urlSerializer: UrlSerializer,
282+
abortSignal: AbortSignal,
280283
): Observable<GuardResult> {
281284
const canMatch = route.canMatch;
282285
if (!canMatch || canMatch.length === 0) return of(true);
@@ -286,7 +289,7 @@ export function runCanMatchGuards(
286289
const guardVal = isCanMatch(guard)
287290
? guard.canMatch(route, segments)
288291
: runInInjectionContext(injector, () => (guard as CanMatchFn)(route, segments));
289-
return wrapIntoObservable(guardVal);
292+
return wrapIntoObservable(guardVal).pipe(takeUntilAbort(abortSignal));
290293
});
291294

292295
return of(canMatchObservables).pipe(prioritizedGuardValue(), redirectIfUrlTree(urlSerializer));

packages/router/src/operators/recognize.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,19 @@ export function recognize(
2323
config: Route[],
2424
serializer: UrlSerializer,
2525
paramsInheritanceStrategy: 'emptyOnly' | 'always',
26+
abortSignal: AbortSignal,
2627
): MonoTypeOperatorFunction<NavigationTransition> {
27-
return mergeMap((t) =>
28-
recognizeFn(
28+
return mergeMap(async (t) => {
29+
const {state: targetSnapshot, tree: urlAfterRedirects} = await recognizeFn(
2930
injector,
3031
configLoader,
3132
rootComponentType,
3233
config,
3334
t.extractedUrl,
3435
serializer,
3536
paramsInheritanceStrategy,
36-
).pipe(
37-
map(({state: targetSnapshot, tree: urlAfterRedirects}) => {
38-
return {...t, targetSnapshot, urlAfterRedirects};
39-
}),
40-
),
41-
);
37+
abortSignal,
38+
);
39+
return {...t, targetSnapshot, urlAfterRedirects};
40+
});
4241
}

0 commit comments

Comments
 (0)