Skip to content

refactor(router): make recognize synchronous to expose it later #30410

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.

Sign up for GitHub

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

Closed
yelhouti wants to merge 1 commit into from
Closed

refactor(router): make recognize synchronous to expose it later #30410

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
refactor(router): make recognize synchronous to expose it later 
make recognize synchronous as suggested by @jasonaden in PR 15826.
This is better since it would allow to convert URLs to snapshot and use
metadata without the need for Observables
  • Loading branch information
@yelhouti
yelhouti committed May 11, 2019
commit 20705956e31f141b510e3258a309e499df89c979
13 changes: 7 additions & 6 deletions packages/router/src/operators/recognize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {Type} from '@angular/core';
import {MonoTypeOperatorFunction, Observable} from 'rxjs';
import {map, mergeMap} from 'rxjs/operators';
import {map} from 'rxjs/operators';

import {Route} from '../config';
import {recognize as recognizeFn} from '../recognize';
Expand All @@ -20,10 +20,11 @@ export function recognize(
paramsInheritanceStrategy: 'emptyOnly' | 'always', relativeLinkResolution: 'legacy' |
'corrected'): MonoTypeOperatorFunction<NavigationTransition> {
return function(source: Observable<NavigationTransition>) {
return source.pipe(mergeMap(
t => recognizeFn(
rootComponentType, config, t.urlAfterRedirects, serializer(t.urlAfterRedirects),
paramsInheritanceStrategy, relativeLinkResolution)
.pipe(map(targetSnapshot => ({...t, targetSnapshot})))));
return source.pipe(map(t => {
const targetSnapshot = recognizeFn(
rootComponentType, config, t.urlAfterRedirects, serializer(t.urlAfterRedirects),
paramsInheritanceStrategy, relativeLinkResolution);
return {...t, targetSnapshot};
}));
};
}
33 changes: 13 additions & 20 deletions packages/router/src/recognize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import {Type} from '@angular/core';
import {Observable, Observer, of } from 'rxjs';

import {Data, ResolveData, Route, Routes} from './config';
import {ActivatedRouteSnapshot, ParamsInheritanceStrategy, RouterStateSnapshot, inheritedParamsDataResolve} from './router_state';
Expand All @@ -21,7 +20,7 @@ class NoMatch {}
export function recognize(
rootComponentType: Type<any>| null, config: Routes, urlTree: UrlTree, url: string,
paramsInheritanceStrategy: ParamsInheritanceStrategy = 'emptyOnly',
relativeLinkResolution: 'legacy' | 'corrected' = 'legacy'): Observable<RouterStateSnapshot> {
relativeLinkResolution: 'legacy' | 'corrected' = 'legacy'): RouterStateSnapshot {
return new Recognizer(
rootComponentType, config, urlTree, url, paramsInheritanceStrategy,
relativeLinkResolution)
Expand All @@ -34,27 +33,21 @@ class Recognizer {
private url: string, private paramsInheritanceStrategy: ParamsInheritanceStrategy,
private relativeLinkResolution: 'legacy'|'corrected') {}

recognize(): Observable<RouterStateSnapshot> {
try {
const rootSegmentGroup =
split(this.urlTree.root, [], [], this.config, this.relativeLinkResolution).segmentGroup;
recognize(): RouterStateSnapshot {
const rootSegmentGroup =
split(this.urlTree.root, [], [], this.config, this.relativeLinkResolution).segmentGroup;

const children = this.processSegmentGroup(this.config, rootSegmentGroup, PRIMARY_OUTLET);
const children = this.processSegmentGroup(this.config, rootSegmentGroup, PRIMARY_OUTLET);

const root = new ActivatedRouteSnapshot(
[], Object.freeze({}), Object.freeze({...this.urlTree.queryParams}),
this.urlTree.fragment !, {}, PRIMARY_OUTLET, this.rootComponentType, null,
this.urlTree.root, -1, {});
const root = new ActivatedRouteSnapshot(
[], Object.freeze({}), Object.freeze({...this.urlTree.queryParams}),
this.urlTree.fragment !, {}, PRIMARY_OUTLET, this.rootComponentType, null,
this.urlTree.root, -1, {});

const rootNode = new TreeNode<ActivatedRouteSnapshot>(root, children);
const routeState = new RouterStateSnapshot(this.url, rootNode);
this.inheritParamsAndData(routeState._root);
return of (routeState);

} catch (e) {

@atscott atscott Aug 6, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this try...catch be moved to operators/recognize.ts to avoid unintentionally throwing errors that can't be handled by catchError?

You also removed the test for the error case below. Maybe add that back and use the recognize operator.

return new Observable<RouterStateSnapshot>(
(obs: Observer<RouterStateSnapshot>) => obs.error(e));
}
const rootNode = new TreeNode<ActivatedRouteSnapshot>(root, children);
const routeState = new RouterStateSnapshot(this.url, rootNode);
this.inheritParamsAndData(routeState._root);
return routeState;
}

inheritParamsAndData(routeNode: TreeNode<ActivatedRouteSnapshot>): void {
Expand Down
4 changes: 1 addition & 3 deletions packages/router/test/create_router_state.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,7 @@ function advanceNode(node: TreeNode<ActivatedRoute>): void {
}

function createState(config: Routes, url: string): RouterStateSnapshot {
let res: RouterStateSnapshot = undefined !;
recognize(RootComponent, config, tree(url), url).forEach(s => res = s);
return res;
return recognize(RootComponent, config, tree(url), url);
}

function checkActivatedRoute(
Expand Down
Loading