Skip to content

Commit

Permalink
refactor(router): Move navigationId handling to the transition manager (
Browse files Browse the repository at this point in the history
#48202)

The navigationId is really just a count of how many navigations have been
processed through the navigation pipeline. This tracking should be
done as part of the navigation transition handler

PR Close #48202
  • Loading branch information
atscott committed Nov 29, 2022
1 parent 55ae4aa commit 0ff5d97
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 19 deletions.
27 changes: 20 additions & 7 deletions packages/router/src/navigation_transition.ts
Expand Up @@ -266,7 +266,6 @@ interface InternalRouterInterface {
browserUrlTree: UrlTree;
currentUrlTree: UrlTree;
rawUrlTree: UrlTree;
navigationId: number;
readonly routerState: RouterState;
errorHandler: ErrorHandler;
titleStrategy?: TitleStrategy;
Expand Down Expand Up @@ -297,10 +296,11 @@ export class NavigationTransitions {
private readonly environmentInjector = inject(EnvironmentInjector);
private readonly urlSerializer = inject(UrlSerializer);
private readonly rootContexts = inject(ChildrenOutletContexts);
private _transitions?: BehaviorSubject<NavigationTransition>;
get transitions(): BehaviorSubject<NavigationTransition>|undefined {
return this._transitions;
navigationId = 0;
get hasRequestedNavigation() {
return this.navigationId !== 0;
}
private transitions?: BehaviorSubject<NavigationTransition>;

constructor() {
const onLoadStart = (r: Route) => this.events.next(new RouteConfigLoadStart(r));
Expand All @@ -309,8 +309,21 @@ export class NavigationTransitions {
this.configLoader.onLoadStartListener = onLoadStart;
}

complete() {
this.transitions?.complete();
}

handleNavigationRequest(
request: Pick<
NavigationTransition,
'targetPageId'|'source'|'restoredState'|'currentUrlTree'|'currentRawUrl'|'rawUrl'|
'extras'|'resolve'|'reject'|'promise'|'currentSnapshot'|'currentRouterState'>) {
const id = ++this.navigationId;
this.transitions?.next({...this.transitions.value, ...request, id});
}

setupNavigations(router: InternalRouterInterface): Observable<NavigationTransition> {
this._transitions = new BehaviorSubject<NavigationTransition>({
this.transitions = new BehaviorSubject<NavigationTransition>({
id: 0,
targetPageId: 0,
currentUrlTree: router.currentUrlTree,
Expand All @@ -331,7 +344,7 @@ export class NavigationTransitions {
guards: {canActivateChecks: [], canDeactivateChecks: []},
guardsResult: null,
});
return this._transitions.pipe(
return this.transitions.pipe(
filter(t => t.id !== 0),

// Extract URL
Expand Down Expand Up @@ -662,7 +675,7 @@ export class NavigationTransitions {
`Navigation ID ${
overallTransitionState
.id} is not equal to the current navigation id ${
router.navigationId}` :
this.navigationId}` :
'';
this.cancelNavigationTransition(
overallTransitionState, cancelationReason,
Expand Down
21 changes: 9 additions & 12 deletions packages/router/src/router.ts
Expand Up @@ -178,8 +178,11 @@ export class Router {
private disposed = false;

private locationSubscription?: SubscriptionLike;
/** @internal */
navigationId: number = 0;
// TODO(b/260747083): This should not exist and navigationId should be private in
// `NavigationTransitions`
private get navigationId() {
return this.navigationTransitions.navigationId;
}

/**
* The id of the currently active page in the router.
Expand Down Expand Up @@ -394,17 +397,12 @@ export class Router {
this.routerState.root.component = this.rootComponentType;
}

private setTransition(t: Partial<NavigationTransition>): void {
this.navigationTransitions.transitions?.next(
{...this.navigationTransitions.transitions?.value, ...t});
}

/**
* Sets up the location change listener and performs the initial navigation.
*/
initialNavigation(): void {
this.setUpLocationChangeListener();
if (this.navigationId === 0) {
if (!this.navigationTransitions.hasRequestedNavigation) {
this.navigateByUrl(this.location.path(true), {replaceUrl: true});
}
}
Expand Down Expand Up @@ -499,7 +497,7 @@ export class Router {

/** Disposes of the router. */
dispose(): void {
this.navigationTransitions.transitions?.complete();
this.navigationTransitions.complete();
if (this.locationSubscription) {
this.locationSubscription.unsubscribe();
this.locationSubscription = undefined;
Expand Down Expand Up @@ -736,7 +734,6 @@ export class Router {
});
}

const id = ++this.navigationId;
let targetPageId: number;
if (this.canceledNavigationResolution === 'computed') {
const isInitialPage = this.currentPageId === 0;
Expand All @@ -762,12 +759,12 @@ export class Router {
targetPageId = 0;
}

this.setTransition({
id,
this.navigationTransitions.handleNavigationRequest({
targetPageId,
source,
restoredState,
currentUrlTree: this.currentUrlTree,
currentRawUrl: this.currentUrlTree,
rawUrl,
extras,
resolve,
Expand Down

0 comments on commit 0ff5d97

Please sign in to comment.