Skip to content

Commit

Permalink
refactor(router): take only the first emitted value of every resolver…
Browse files Browse the repository at this point in the history
… to make it consistent with guards (#44573)

The router used to wait for the resolvers to complete and take the last
value. The changes here take only the first
emitted value of every resolver and proceed the navigation. This matches
how other guards work in the `Router` code.

Resolves #44643

BREAKING CHANGE: Previously, resolvers were waiting to be completed
before proceeding with the navigation and the Router would take the last
value emitted from the resolver.
The router now takes only the first emitted value by the resolvers
and then proceeds with navigation. This is now consistent with `Observables`
returned by other guards: only the first value is used.

PR Close #44573
  • Loading branch information
dimakuba authored and thePunderWoman committed Mar 1, 2022
1 parent bd12f9e commit c967976
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
ActivatedRouteSnapshot
} from '@angular/router';
import { Observable, of, EMPTY } from 'rxjs';
import { mergeMap, take } from 'rxjs/operators';
import { mergeMap } from 'rxjs/operators';

import { CrisisService } from './crisis.service';
import { Crisis } from './crisis';
Expand All @@ -21,7 +21,6 @@ export class CrisisDetailResolverService implements Resolve<Crisis> {
const id = route.paramMap.get('id')!;

return this.cs.getCrisis(id).pipe(
take(1),
mergeMap(crisis => {
if (crisis) {
return of(crisis);
Expand Down
7 changes: 2 additions & 5 deletions aio/content/guide/router-tutorial-toh.md
Original file line number Diff line number Diff line change
Expand Up @@ -1921,9 +1921,9 @@ These are all asynchronous operations.
Accordingly, a routing guard can return an `Observable<boolean>` or a `Promise<boolean>` and the
router will wait for the observable to resolve to `true` or `false`.

<div class="alert is-critical">
<div class="alert is-helpful">

**Note:** The observable provided to the `Router` must also complete. If the observable does not complete, the navigation does not continue.
**Note:** The observable provided to the `Router` automatically completes after retrieving the first value.

</div>

Expand Down Expand Up @@ -2414,9 +2414,6 @@ Inject the `CrisisService` and `Router` and implement the `resolve()` method.
That method could return a `Promise`, an `Observable`, or a synchronous return value.

The `CrisisService.getCrisis()` method returns an observable in order to prevent the route from loading until the data is fetched.
The `Router` guards require an observable to `complete`, which means it has emitted all
of its values.
You use the `take` operator with an argument of `1` to ensure that the `Observable` completes after retrieving the first value from the Observable returned by the `getCrisis()` method.

If it doesn't return a valid `Crisis`, then return an empty `Observable`, cancel the previous in-progress navigation to the `CrisisDetailComponent`, and navigate the user back to the `CrisisListComponent`.
The updated resolver service looks like this:
Expand Down
8 changes: 4 additions & 4 deletions packages/router/src/operators/resolve_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Injector} from '@angular/core';
import {EMPTY, from, MonoTypeOperatorFunction, Observable, of} from 'rxjs';
import {concatMap, map, mergeMap, takeLast, tap} from 'rxjs/operators';
import {concatMap, map, mergeMap, take, takeLast, tap} from 'rxjs/operators';

import {ResolveData} from '../models';
import {NavigationTransition} from '../router';
Expand Down Expand Up @@ -80,9 +80,9 @@ function resolveNode(
return from(keys).pipe(
mergeMap(
key => getResolver(resolve[key], futureARS, futureRSS, moduleInjector)
.pipe(tap((value: any) => {
data[key] = value;
}))),
.pipe(take(1), tap((value: any) => {
data[key] = value;
}))),
takeLast(1),
mergeMap(() => {
// Ensure all resolvers returned values, otherwise don't emit any "next" and just complete
Expand Down
17 changes: 16 additions & 1 deletion packages/router/test/operators/resolve_data.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Injector} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {EMPTY, of} from 'rxjs';
import {EMPTY, interval, of} from 'rxjs';
import {TestScheduler} from 'rxjs/testing';

import {resolveData} from '../../src/operators/resolve_data';
Expand All @@ -23,6 +23,7 @@ describe('resolveData operator', () => {
{provide: 'resolveTwo', useValue: (a: any, b: any) => of(2)},
{provide: 'resolveFour', useValue: (a: any, b: any) => 4},
{provide: 'resolveEmpty', useValue: (a: any, b: any) => EMPTY},
{provide: 'resolveInterval', useValue: (a: any, b: any) => interval()},
]
});
});
Expand All @@ -48,6 +49,20 @@ describe('resolveData operator', () => {
});
});

it('should take only the first emitted value of every resolver', () => {
testScheduler.run(({cold, expectObservable}) => {
const transition: any = createTransition({e1: 'resolveInterval'});
const source = cold('-(t|)', {t: deepClone(transition)});
const expected = '-(t|)';
const outputTransition = deepClone(transition);
outputTransition.guards.canActivateChecks[0].route._resolvedData = {e1: 0};

expectObservable(source.pipe(resolveData('emptyOnly', injector))).toBe(expected, {
t: outputTransition
});
});
});

it('should re-emit value from source when there are no resolvers', () => {
testScheduler.run(({hot, cold, expectObservable}) => {
const transition: any = createTransition({});
Expand Down

0 comments on commit c967976

Please sign in to comment.