Skip to content
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

feat(router): add prioritizedGuardValue operator optimization and allowing UrlTree return from guard #26478

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
52 changes: 52 additions & 0 deletions packages/router/src/operators/prioritized_guard_value.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/

import {Observable, OperatorFunction, combineLatest} from 'rxjs';
import {filter, scan, startWith, switchMap, take} from 'rxjs/operators';

import {UrlTree} from '../url_tree';

const INITIAL_VALUE = Symbol('INITIAL_VALUE');
declare type INTERIM_VALUES = typeof INITIAL_VALUE | boolean | UrlTree;

export function prioritizedGuardValue():
OperatorFunction<Observable<boolean|UrlTree>[], boolean|UrlTree> {
return switchMap(obs => {
return combineLatest(
...obs.map(o => o.pipe(take(1), startWith(INITIAL_VALUE as INTERIM_VALUES))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: o => o.pipe(take(1), startWith(INITIAL_VALUE as INTERIM_VALUES))) could be pulled out into it's own static function for better performance and lower memory pressure. (Also might help readability)

.pipe(
scan(
(acc: INTERIM_VALUES, list: INTERIM_VALUES[]) => {
let isPending = false;
return list.reduce((innerAcc, val, i: number) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: The reducer function inside of here could be pulled out into it's own static function (like above)

if (innerAcc !== INITIAL_VALUE) return innerAcc;

// Toggle pending flag if any values haven't been set yet
if (val === INITIAL_VALUE) isPending = true;

// Any other return values are only valid if we haven't yet hit a pending call.
// This guarantees that in the case of a guard at the bottom of the tree that
// returns a redirect, we will wait for the higher priority guard at the top to
// finish before performing the redirect.
if (!isPending) {
// Early return when we hit a `false` value as that should always cancel
// navigation
if (val === false) return val;

if (i === list.length - 1 || val instanceof UrlTree) {
return val;
}
}

return innerAcc;
}, acc);
},
INITIAL_VALUE),
filter(item => item !== INITIAL_VALUE), take(1)) as Observable<boolean|UrlTree>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Adding a trailing comma will make clang format this cleaner.

});
}
1 change: 1 addition & 0 deletions packages/router/test/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ ts_library(
"//packages/router/testing",
"@rxjs",
"@rxjs//operators",
"@rxjs//testing",
],
)

Expand Down
182 changes: 182 additions & 0 deletions packages/router/test/operators/prioritized_guard_value.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/


import {TestBed} from '@angular/core/testing';
import {Observable, Observer, of } from 'rxjs';
import {every, mergeMap} from 'rxjs/operators';
import {TestScheduler} from 'rxjs/testing';

import {prioritizedGuardValue} from '../../src/operators/prioritized_guard_value';
import {Router} from '../../src/router';
import {UrlTree} from '../../src/url_tree';
import {RouterTestingModule} from '../../testing/src/router_testing_module';


describe('prioritizedGuardValue operator', () => {
let testScheduler: TestScheduler;
let router: Router;
const TF = {T: true, F: false};

beforeEach(() => { TestBed.configureTestingModule({imports: [RouterTestingModule]}); });
beforeEach(() => { testScheduler = new TestScheduler(assertDeepEquals); });
beforeEach(() => { router = TestBed.get(Router); });

it('should return true if all values are true', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const a = cold(' --(T|)', TF);
const b = cold(' ----------(T|)', TF);
const c = cold(' ------(T|)', TF);
const source = hot('---o--', {o: [a, b, c]});

const expected = ' -------------T--';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, TF, /* an error here maybe */);
});
});

it('should return false if observables to the left of false have produced a value', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const a = cold(' --(T|)', TF);
const b = cold(' ----------(T|)', TF);
const c = cold(' ------(F|)', TF);
const source = hot('---o--', {o: [a, b, c]});

const expected = ' -------------F--';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, TF, /* an error here maybe */);
});
});

it('should ignore results for unresolved sets of Observables', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const a = cold(' --(T|)', TF);
const b = cold(' -------------(T|)', TF);
const c = cold(' ------(F|)', TF);

const z = cold(' ----(T|)', TF);

const source = hot('---o----p----', {o: [a, b, c], p: [z]});

const expected = ' ------------T---';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, TF, /* an error here maybe */);
});
});

it('should return UrlTree if higher priority guards have resolved', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const urlTree = router.parseUrl('/');

const urlLookup = {U: urlTree};

const a = cold(' --(T|)', TF);
const b = cold(' ----------(U|)', urlLookup);
const c = cold(' ------(T|)', TF);

const source = hot('---o---', {o: [a, b, c]});

const expected = ' -------------U---';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, urlLookup, /* an error here maybe */);
});
});

it('should return false even with UrlTree if UrlTree is lower priority', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const urlTree = router.parseUrl('/');

const urlLookup = {U: urlTree};

const a = cold(' --(T|)', TF);
const b = cold(' ----------(F|)', TF);
const c = cold(' ------(U|)', urlLookup);

const source = hot('---o---', {o: [a, b, c]});

const expected = ' -------------F---';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, TF, /* an error here maybe */);
});
});

it('should return UrlTree even after a false if the false is lower priority', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const urlTree = router.parseUrl('/');

const urlLookup = {U: urlTree};

const a = cold(' --(T|)', TF);
const b = cold(' ----------(U|)', urlLookup);
const c = cold(' ------(F|)', TF);

const source = hot('---o---', {o: [a, b, c]});

const expected = ' -------------U----';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, urlLookup, /* an error here maybe */);
});
});

it('should return the highest priority UrlTree', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const urlTreeU = router.parseUrl('/u');
const urlTreeR = router.parseUrl('/r');
const urlTreeL = router.parseUrl('/l');

const urlLookup = {U: urlTreeU, R: urlTreeR, L: urlTreeL};

const a = cold(' ----------(U|)', urlLookup);
const b = cold(' -----(R|)', urlLookup);
const c = cold(' --(L|)', urlLookup);

const source = hot('---o---', {o: [a, b, c]});

const expected = ' -------------U---';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, urlLookup, /* an error here maybe */);
});
});

it('should propagate errors', () => {
testScheduler.run(({hot, cold, expectObservable}) => {

const a = cold(' --(T|)', TF);
const b = cold(' ------#', TF);
const c = cold(' ----------(F|)', TF);
const source = hot('---o------', {o: [a, b, c]});

const expected = ' ---------#';

expectObservable(source.pipe(prioritizedGuardValue()))
.toBe(expected, TF, /* an error here maybe */);
});
});


});



function assertDeepEquals(a: any, b: any) {
return expect(a).toEqual(b);
}