Skip to content

Commit

Permalink
feat(router): Add ability to return UrlTree with `NavigationBehavio…
Browse files Browse the repository at this point in the history
…rOptions` from guards (#45023)

Returning `UrlTree` from a guard was a convenient new feature added to
the `Router`. However, it does not have feature-parity with the old
`router.navigate(...); return false;` pattern. The most common use-case
for this feature is to redirect to a new page _without_ updating the URL
from the initially attempted navigation. For example, rendering a 404
page when the user does not have access privelages to a route.

Fixes #17004
Fixes #27148

BREAKING CHANGE: Guards can now return `RedirectCommand` for redirects
in addition to `UrlTree`. Code which expects only `boolean` or `UrlTree`
values in `Route` types will need to be adjusted.

PR Close #45023
  • Loading branch information
atscott committed Mar 12, 2024
1 parent 87cae55 commit 8735af0
Show file tree
Hide file tree
Showing 13 changed files with 124 additions and 26 deletions.
2 changes: 1 addition & 1 deletion adev/src/content/guide/routing/router-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ These events are shown in the following table.
| [`ChildActivationEnd`](api/router/ChildActivationEnd) | Triggered when the Router finishes activating a route's children. |
| [`ActivationEnd`](api/router/ActivationEnd) | Triggered when the Router finishes activating a route. |
| [`NavigationEnd`](api/router/NavigationEnd) | Triggered when navigation ends successfully. |
| [`NavigationCancel`](api/router/NavigationCancel) | Triggered when navigation is canceled. This can happen when a Route Guard returns false during navigation, or redirects by returning a `UrlTree`. |
| [`NavigationCancel`](api/router/NavigationCancel) | Triggered when navigation is canceled. This can happen when a Route Guard returns false during navigation, or redirects by returning a `UrlTree` or `RedirectCommand`. |
| [`NavigationError`](api/router/NavigationError) | Triggered when navigation fails due to an unexpected error. |
| [`Scroll`](api/router/Scroll) | Represents a scrolling event. |

Expand Down
11 changes: 10 additions & 1 deletion goldens/public-api/router/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,7 @@ export interface ExtraOptions extends InMemoryScrollingOptions, RouterConfigOpti
}

// @public
export type GuardResult = boolean | UrlTree;
export type GuardResult = boolean | UrlTree | RedirectCommand;

// @public
export class GuardsCheckEnd extends RouterEvent {
Expand Down Expand Up @@ -595,6 +595,15 @@ export function provideRoutes(routes: Routes): Provider[];
// @public
export type QueryParamsHandling = 'merge' | 'preserve' | '';

// @public
export class RedirectCommand {
constructor(redirectTo: UrlTree, navigationBehaviorOptions?: NavigationBehaviorOptions | undefined);
// (undocumented)
readonly navigationBehaviorOptions?: NavigationBehaviorOptions | undefined;
// (undocumented)
readonly redirectTo: UrlTree;
}

// @public @deprecated
export interface Resolve<T> {
// (undocumented)
Expand Down
3 changes: 3 additions & 0 deletions packages/core/test/bundling/router/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,9 @@
{
"name": "Recognizer"
},
{
"name": "RedirectCommand"
},
{
"name": "RedirectRequest"
},
Expand Down
7 changes: 5 additions & 2 deletions packages/router/src/events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Route} from './models';
import {NavigationBehaviorOptions, Route} from './models';
import {ActivatedRouteSnapshot, RouterStateSnapshot} from './router_state';
import {UrlTree} from './url_tree';

Expand Down Expand Up @@ -611,7 +611,10 @@ export class Scroll {

export class BeforeActivateRoutes {}
export class RedirectRequest {
constructor(readonly url: UrlTree) {}
constructor(
readonly url: UrlTree,
readonly navigationBehaviorOptions: NavigationBehaviorOptions | undefined,
) {}
}
export type PrivateRouterEvents = BeforeActivateRoutes | RedirectRequest;

Expand Down
1 change: 1 addition & 0 deletions packages/router/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export {
RunGuardsAndResolvers,
UrlMatcher,
UrlMatchResult,
RedirectCommand,
} from './models';
export {ViewTransitionInfo, ViewTransitionsFeatureOptions} from './utils/view_transition';

Expand Down
18 changes: 17 additions & 1 deletion packages/router/src/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,23 @@ export type DeprecatedGuard = ProviderToken<any> | any;
* @see [Routing tutorial](guide/router-tutorial-toh#milestone-5-route-guards)
* @publicApi
*/
export type GuardResult = boolean | UrlTree;
export type GuardResult = boolean | UrlTree | RedirectCommand;

/**
* Can be returned by a `Router` guard to instruct the `Router` to redirect rather than continue
* processing the path of the in-flight navigation. The `redirectTo` indicates _where_ the new
* navigation should go to and the optional `navigationBehaviorOptions` can provide more information
* about _how_ to perform the navigation.
*
* @see [Routing tutorial](guide/router-tutorial-toh#milestone-5-route-guards)
* @publicApi
*/
export class RedirectCommand {
constructor(
readonly redirectTo: UrlTree,
readonly navigationBehaviorOptions?: NavigationBehaviorOptions,
) {}
}

/**
* Type used to represent a value which may be synchronous or async.
Expand Down
4 changes: 2 additions & 2 deletions packages/router/src/navigation_canceling_error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import {NavigationCancellationCode} from './events';
import {NavigationBehaviorOptions} from './models';
import {NavigationBehaviorOptions, RedirectCommand} from './models';
import {isUrlTree, UrlSerializer, UrlTree} from './url_tree';

export const NAVIGATION_CANCELING_ERROR = 'ngNavigationCancelingError';
Expand All @@ -24,7 +24,7 @@ export type RedirectingNavigationCancelingError = NavigationCancelingError & {

export function redirectingNavigationError(
urlSerializer: UrlSerializer,
redirect: UrlTree,
redirect: UrlTree | RedirectCommand,
): RedirectingNavigationCancelingError {
const {redirectTo, navigationBehaviorOptions} = isUrlTree(redirect)
? {redirectTo: redirect, navigationBehaviorOptions: undefined}
Expand Down
8 changes: 4 additions & 4 deletions packages/router/src/navigation_transition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ import {
RouteConfigLoadStart,
RoutesRecognized,
} from './events';
import {NavigationBehaviorOptions, QueryParamsHandling, Route, Routes} from './models';
import {GuardResult, NavigationBehaviorOptions, QueryParamsHandling, Route, Routes} from './models';
import {
isNavigationCancelingError,
isRedirectingNavigationCancelingError,
Expand Down Expand Up @@ -297,7 +297,7 @@ export interface NavigationTransition {
currentRouterState: RouterState;
targetRouterState: RouterState | null;
guards: Checks;
guardsResult: boolean | UrlTree | null;
guardsResult: GuardResult | null;
}

/**
Expand Down Expand Up @@ -608,7 +608,7 @@ export class NavigationTransitions {
checkGuards(this.environmentInjector, (evt: Event) => this.events.next(evt)),
tap((t) => {
overallTransitionState.guardsResult = t.guardsResult;
if (isUrlTree(t.guardsResult)) {
if (t.guardsResult && typeof t.guardsResult !== 'boolean') {
throw redirectingNavigationError(this.urlSerializer, t.guardsResult);
}

Expand Down Expand Up @@ -824,7 +824,7 @@ export class NavigationTransitions {
if (!isRedirectingNavigationCancelingError(e)) {
overallTransitionState.resolve(false);
} else {
this.events.next(new RedirectRequest(e.url));
this.events.next(new RedirectRequest(e.url, e.navigationBehaviorOptions));
}

/* All other errors should reset to the router's internal URL reference
Expand Down
12 changes: 5 additions & 7 deletions packages/router/src/operators/check_guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import {
CanMatchFn,
Route,
} from '../models';
import {redirectingNavigationError} from '../navigation_canceling_error';
import {navigationCancelingError, redirectingNavigationError} from '../navigation_canceling_error';
import {NavigationTransition} from '../navigation_transition';
import {ActivatedRouteSnapshot, RouterStateSnapshot} from '../router_state';
import {isUrlTree, UrlSegment, UrlSerializer, UrlTree} from '../url_tree';
Expand Down Expand Up @@ -260,12 +260,10 @@ export function runCanLoadGuards(
return of(canLoadObservables).pipe(prioritizedGuardValue(), redirectIfUrlTree(urlSerializer));
}

function redirectIfUrlTree(
urlSerializer: UrlSerializer,
): OperatorFunction<UrlTree | boolean, boolean> {
function redirectIfUrlTree(urlSerializer: UrlSerializer): OperatorFunction<GuardResult, boolean> {
return pipe(
tap((result: UrlTree | boolean) => {
if (!isUrlTree(result)) return;
tap((result: GuardResult) => {
if (typeof result === 'boolean') return;

throw redirectingNavigationError(urlSerializer, result);
}),
Expand All @@ -278,7 +276,7 @@ export function runCanMatchGuards(
route: Route,
segments: UrlSegment[],
urlSerializer: UrlSerializer,
): Observable<boolean> {
): Observable<GuardResult> {
const canMatch = route.canMatch;
if (!canMatch || canMatch.length === 0) return of(true);

Expand Down
14 changes: 9 additions & 5 deletions packages/router/src/operators/prioritized_guard_value.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
import {combineLatest, Observable, OperatorFunction} from 'rxjs';
import {filter, map, startWith, switchMap, take} from 'rxjs/operators';

import {GuardResult} from '../models';
import {UrlTree} from '../url_tree';
import {GuardResult, RedirectCommand} from '../models';
import {isUrlTree, UrlTree} from '../url_tree';

const INITIAL_VALUE = /* @__PURE__ */ Symbol('INITIAL_VALUE');
declare type INTERIM_VALUES = typeof INITIAL_VALUE | boolean | UrlTree;
declare type INTERIM_VALUES = typeof INITIAL_VALUE | GuardResult;

export function prioritizedGuardValue(): OperatorFunction<Observable<GuardResult>[], GuardResult> {
return switchMap((obs) => {
Expand All @@ -28,9 +28,9 @@ export function prioritizedGuardValue(): OperatorFunction<Observable<GuardResult
} else if (result === INITIAL_VALUE) {
// If guard has not finished, we need to stop processing.
return INITIAL_VALUE;
} else if (result === false || result instanceof UrlTree) {
} else if (result === false || isRedirect(result)) {
// Result finished and was not true. Return the result.
// Note that we only allow false/UrlTree. Other values are considered invalid and
// Note that we only allow false/UrlTree/RedirectCommand. Other values are considered invalid and
// ignored.
return result;
}
Expand All @@ -43,3 +43,7 @@ export function prioritizedGuardValue(): OperatorFunction<Observable<GuardResult
);
});
}

function isRedirect(val: INTERIM_VALUES): val is UrlTree | RedirectCommand {
return isUrlTree(val) || val instanceof RedirectCommand;
}
3 changes: 3 additions & 0 deletions packages/router/src/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ export class Router {
} else if (e instanceof NavigationEnd) {
this.navigated = true;
} else if (e instanceof RedirectRequest) {
const opts = e.navigationBehaviorOptions;
const mergedTree = this.urlHandlingStrategy.merge(
e.url,
currentTransition.currentRawUrl,
Expand All @@ -235,6 +236,8 @@ export class Router {
replaceUrl:
this.urlUpdateStrategy === 'eager' ||
isBrowserTriggeredNavigation(currentTransition.source),
// allow developer to override default options with RedirectCommand
...opts,
};

this.scheduleNavigation(mergedTree, IMPERATIVE_NAVIGATION, null, extras, {
Expand Down
63 changes: 62 additions & 1 deletion packages/router/test/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,16 @@ import {RouterTestingHarness} from '@angular/router/testing';
import {concat, EMPTY, firstValueFrom, Observable, Observer, of, Subscription} from 'rxjs';
import {delay, filter, first, last, map, mapTo, takeWhile, tap} from 'rxjs/operators';

import {CanActivateChildFn, CanActivateFn, CanMatchFn, Data, ResolveFn} from '../src/models';
import {
CanActivateChildFn,
CanActivateFn,
CanMatchFn,
Data,
ResolveFn,
RedirectCommand,
GuardResult,
MaybeAsync,
} from '../src/models';
import {provideRouter, withNavigationErrorHandler, withRouterConfig} from '../src/provide_router';
import {wrapIntoObservable} from '../src/utils/collection';
import {getLoadedRoutes} from '../src/utils/config';
Expand Down Expand Up @@ -3959,6 +3968,58 @@ for (const browserAPI of ['navigation', 'history'] as const) {
tick();
expect(resolvedPath).toBe('/redirected');
}));

it('can redirect to 404 without changing the URL', fakeAsync(() => {
TestBed.configureTestingModule({
providers: [provideRouter([], withRouterConfig({urlUpdateStrategy: 'eager'}))],
});
const router = TestBed.inject(Router);
const location = TestBed.inject(Location);
router.resetConfig([
{path: '', component: SimpleCmp},
{
path: 'one',
component: RouteCmp,
canActivate: [
() => new RedirectCommand(router.parseUrl('/404'), {skipLocationChange: true}),
],
},
{path: '404', component: SimpleCmp},
]);
const fixture = createRoot(router, RootCmp);
router.navigateByUrl('/one');

advance(fixture);

expect(location.path()).toEqual('/one');
expect(router.url.toString()).toEqual('/404');
}));

it('can redirect while changing state object', fakeAsync(() => {
TestBed.configureTestingModule({
providers: [provideRouter([], withRouterConfig({urlUpdateStrategy: 'eager'}))],
});
const router = TestBed.inject(Router);
const location = TestBed.inject(Location);
router.resetConfig([
{path: '', component: SimpleCmp},
{
path: 'one',
component: RouteCmp,
canActivate: [
() => new RedirectCommand(router.parseUrl('/redirected'), {state: {test: 1}}),
],
},
{path: 'redirected', component: SimpleCmp},
]);
const fixture = createRoot(router, RootCmp);
router.navigateByUrl('/one');

advance(fixture);

expect(location.path()).toEqual('/redirected');
expect(location.getState()).toEqual(jasmine.objectContaining({test: 1}));
}));
});

describe('runGuardsAndResolvers', () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/router/test/router.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {RouterModule} from '@angular/router';
import {of} from 'rxjs';

import {ChildActivationStart} from '../src/events';
import {Routes} from '../src/models';
import {GuardResult, Routes} from '../src/models';
import {NavigationTransition} from '../src/navigation_transition';
import {checkGuards as checkGuardsOperator} from '../src/operators/check_guards';
import {resolveData as resolveDataOperator} from '../src/operators/resolve_data';
Expand Down Expand Up @@ -853,7 +853,7 @@ function checkGuards(
future: RouterStateSnapshot,
curr: RouterStateSnapshot,
injector: EnvironmentInjector,
check: (result: boolean | UrlTree) => void,
check: (result: GuardResult) => void,
): void {
// Since we only test the guards, we don't need to provide a full navigation
// transition object with all properties set.
Expand Down

0 comments on commit 8735af0

Please sign in to comment.