From d180db15bf7aef83335ec152002e58a1d9e3031e Mon Sep 17 00:00:00 2001 From: Andrew Scott Date: Tue, 8 Mar 2022 10:42:39 -0800 Subject: [PATCH] refactor(router): Add warning for `relativeLinkResolution: 'legacy'` (#45523) This change adds code to compute the corrected value for a link, regardless of the `relativeLinkResolution` value. Then, if the `relativeLinkResolution` is set to `legacy` and differs from the correct value, a warning is printed to the console in dev mode. This change is meant to assist in notifying developers that they have code which relies on the deprecated, broken behavior so they can fix and update the code before the `relativeLinkResolution` option is fully removed. PR Close #45523 --- aio/content/guide/deprecations.md | 2 + packages/router/src/create_url_tree.ts | 41 ++++++++++++++------ packages/router/src/recognize.ts | 31 ++++++++++++--- packages/router/src/router_state.ts | 10 ++++- packages/router/src/url_tree.ts | 7 ++++ packages/router/src/utils/config_matching.ts | 3 ++ packages/router/test/integration.spec.ts | 10 +++++ 7 files changed, 85 insertions(+), 19 deletions(-) diff --git a/aio/content/guide/deprecations.md b/aio/content/guide/deprecations.md index 5fe705c694d..6cd7d2f264b 100644 --- a/aio/content/guide/deprecations.md +++ b/aio/content/guide/deprecations.md @@ -321,6 +321,8 @@ The `relativeLinkResolution` option is deprecated and being removed. In version 11, the default behavior was changed to the correct one. After `relativeLinkResolution` is removed, the correct behavior is always used without an option to use the broken behavior. +A dev mode warning was added in v14 to warn if a created `UrlTree` relies on the `relativeLinkResolution: 'legacy'` option. + ### loadChildren string syntax diff --git a/packages/router/src/create_url_tree.ts b/packages/router/src/create_url_tree.ts index a074bce1e11..f9c7a7df621 100644 --- a/packages/router/src/create_url_tree.ts +++ b/packages/router/src/create_url_tree.ts @@ -24,13 +24,30 @@ export function createUrlTree( return tree(urlTree.root, urlTree.root, new UrlSegmentGroup([], {}), queryParams, fragment); } - const startingPosition = findStartingPosition(nav, urlTree, route); + function createTreeUsingPathIndex(lastPathIndex: number) { + const startingPosition = + findStartingPosition(nav, urlTree, route.snapshot._urlSegment, lastPathIndex); - const segmentGroup = startingPosition.processChildren ? - updateSegmentGroupChildren( - startingPosition.segmentGroup, startingPosition.index, nav.commands) : - updateSegmentGroup(startingPosition.segmentGroup, startingPosition.index, nav.commands); - return tree(urlTree.root, startingPosition.segmentGroup, segmentGroup, queryParams, fragment); + const segmentGroup = startingPosition.processChildren ? + updateSegmentGroupChildren( + startingPosition.segmentGroup, startingPosition.index, nav.commands) : + updateSegmentGroup(startingPosition.segmentGroup, startingPosition.index, nav.commands); + return tree(urlTree.root, startingPosition.segmentGroup, segmentGroup, queryParams, fragment); + } + const result = createTreeUsingPathIndex(route.snapshot._lastPathIndex); + + // Check if application is relying on `relativeLinkResolution: 'legacy'` + if (typeof ngDevMode === 'undefined' || !!ngDevMode) { + const correctedResult = createTreeUsingPathIndex(route.snapshot._correctedLastPathIndex); + if (correctedResult.toString() !== result.toString()) { + console.warn( + `relativeLinkResolution: 'legacy' is deprecated and will be removed in a future version of Angular. The link to ${ + result.toString()} will change to ${ + correctedResult.toString()} if the code is not updated before then.`); + } + } + + return result; } function isMatrixParams(command: any): boolean { @@ -151,13 +168,14 @@ class Position { } } -function findStartingPosition(nav: Navigation, tree: UrlTree, route: ActivatedRoute): Position { +function findStartingPosition( + nav: Navigation, tree: UrlTree, segmentGroup: UrlSegmentGroup, + lastPathIndex: number): Position { if (nav.isAbsolute) { return new Position(tree.root, true, 0); } - if (route.snapshot._lastPathIndex === -1) { - const segmentGroup = route.snapshot._urlSegment; + if (lastPathIndex === -1) { // Pathless ActivatedRoute has _lastPathIndex === -1 but should not process children // see issue #26224, #13011, #35687 // However, if the ActivatedRoute is the root we should process children like above. @@ -166,9 +184,8 @@ function findStartingPosition(nav: Navigation, tree: UrlTree, route: ActivatedRo } const modifier = isMatrixParams(nav.commands[0]) ? 0 : 1; - const index = route.snapshot._lastPathIndex + modifier; - return createPositionApplyingDoubleDots( - route.snapshot._urlSegment, index, nav.numberOfDoubleDots); + const index = lastPathIndex + modifier; + return createPositionApplyingDoubleDots(segmentGroup, index, nav.numberOfDoubleDots); } function createPositionApplyingDoubleDots( diff --git a/packages/router/src/recognize.ts b/packages/router/src/recognize.ts index 0bf5194c8ae..5975ffa4b14 100644 --- a/packages/router/src/recognize.ts +++ b/packages/router/src/recognize.ts @@ -18,6 +18,8 @@ import {getOutlet, sortByMatchingOutlets} from './utils/config'; import {isImmediateMatch, match, noLeftoversInUrl, split} from './utils/config_matching'; import {TreeNode} from './utils/tree'; +const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode; + class NoMatch {} function newObservableError(e: unknown): Observable { @@ -159,11 +161,16 @@ export class Recognizer { if (route.path === '**') { const params = segments.length > 0 ? last(segments)!.parameters : {}; + const pathIndexShift = getPathIndexShift(rawSegment) + segments.length; snapshot = new ActivatedRouteSnapshot( segments, params, Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment, getData(route), getOutlet(route), route.component!, route, - getSourceSegmentGroup(rawSegment), getPathIndexShift(rawSegment) + segments.length, - getResolve(route)); + getSourceSegmentGroup(rawSegment), pathIndexShift, getResolve(route), + // NG_DEV_MODE is used to prevent the getCorrectedPathIndexShift function from affecting + // production bundle size. This value is intended only to surface a warning to users + // depending on `relativeLinkResolution: 'legacy'` in dev mode. + (NG_DEV_MODE ? getCorrectedPathIndexShift(rawSegment) + segments.length : + pathIndexShift)); } else { const result = match(rawSegment, route, segments); if (!result.matched) { @@ -171,12 +178,14 @@ export class Recognizer { } consumedSegments = result.consumedSegments; remainingSegments = result.remainingSegments; + const pathIndexShift = getPathIndexShift(rawSegment) + consumedSegments.length; snapshot = new ActivatedRouteSnapshot( consumedSegments, result.parameters, Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment, getData(route), getOutlet(route), route.component!, route, - getSourceSegmentGroup(rawSegment), - getPathIndexShift(rawSegment) + consumedSegments.length, getResolve(route)); + getSourceSegmentGroup(rawSegment), pathIndexShift, getResolve(route), + (NG_DEV_MODE ? getCorrectedPathIndexShift(rawSegment) + consumedSegments.length : + pathIndexShift)); } const childConfig: Route[] = getChildConfig(route); @@ -303,10 +312,20 @@ function getSourceSegmentGroup(segmentGroup: UrlSegmentGroup): UrlSegmentGroup { function getPathIndexShift(segmentGroup: UrlSegmentGroup): number { let s = segmentGroup; - let res = (s._segmentIndexShift ? s._segmentIndexShift : 0); + let res = s._segmentIndexShift ?? 0; while (s._sourceSegment) { s = s._sourceSegment; - res += (s._segmentIndexShift ? s._segmentIndexShift : 0); + res += s._segmentIndexShift ?? 0; + } + return res - 1; +} + +function getCorrectedPathIndexShift(segmentGroup: UrlSegmentGroup): number { + let s = segmentGroup; + let res = s._segmentIndexShiftCorrected ?? s._segmentIndexShift ?? 0; + while (s._sourceSegment) { + s = s._sourceSegment; + res += s._segmentIndexShiftCorrected ?? s._segmentIndexShift ?? 0; } return res - 1; } diff --git a/packages/router/src/router_state.ts b/packages/router/src/router_state.ts index 67151111556..a5848868785 100644 --- a/packages/router/src/router_state.ts +++ b/packages/router/src/router_state.ts @@ -284,6 +284,13 @@ export class ActivatedRouteSnapshot { _urlSegment: UrlSegmentGroup; /** @internal */ _lastPathIndex: number; + /** + * @internal + * + * Used only in dev mode to detect if application relies on `relativeLinkResolution: 'legacy'` + * Should be removed in v16. + */ + _correctedLastPathIndex: number; /** @internal */ _resolve: ResolveData; /** @internal */ @@ -333,10 +340,11 @@ export class ActivatedRouteSnapshot { public outlet: string, /** The component of the route */ public component: Type|string|null, routeConfig: Route|null, urlSegment: UrlSegmentGroup, - lastPathIndex: number, resolve: ResolveData) { + lastPathIndex: number, resolve: ResolveData, correctedLastPathIndex?: number) { this.routeConfig = routeConfig; this._urlSegment = urlSegment; this._lastPathIndex = lastPathIndex; + this._correctedLastPathIndex = correctedLastPathIndex ?? lastPathIndex; this._resolve = resolve; } diff --git a/packages/router/src/url_tree.ts b/packages/router/src/url_tree.ts index 09774ab443a..14f55d1b795 100644 --- a/packages/router/src/url_tree.ts +++ b/packages/router/src/url_tree.ts @@ -227,6 +227,13 @@ export class UrlSegmentGroup { _sourceSegment?: UrlSegmentGroup; /** @internal */ _segmentIndexShift?: number; + /** + * @internal + * + * Used only in dev mode to detect if application relies on `relativeLinkResolution: 'legacy'` + * Should be removed in when `relativeLinkResolution` is removed. + */ + _segmentIndexShiftCorrected?: number; /** The parent node in the url tree */ parent: UrlSegmentGroup|null = null; diff --git a/packages/router/src/utils/config_matching.ts b/packages/router/src/utils/config_matching.ts index 9967e192a25..482c296135b 100644 --- a/packages/router/src/utils/config_matching.ts +++ b/packages/router/src/utils/config_matching.ts @@ -111,6 +111,9 @@ function addEmptyPathsToChildrenIfNeeded( s._sourceSegment = segmentGroup; if (relativeLinkResolution === 'legacy') { s._segmentIndexShift = segmentGroup.segments.length; + if (typeof ngDevMode === 'undefined' || !!ngDevMode) { + s._segmentIndexShiftCorrected = consumedSegments.length; + } } else { s._segmentIndexShift = consumedSegments.length; } diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index f22c24866b4..5730911f836 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -87,6 +87,7 @@ describe('Integration', () => { it('should not ignore empty paths in legacy mode', fakeAsync(inject([Router], (router: Router) => { + const warnSpy = spyOn(console, 'warn'); router.relativeLinkResolution = 'legacy'; const fixture = createRoot(router, RootCmp); @@ -96,6 +97,10 @@ describe('Integration', () => { const link = fixture.nativeElement.querySelector('a'); expect(link.getAttribute('href')).toEqual('/foo/bar/simple'); + expect(warnSpy.calls.first().args[0]) + .toContain('/foo/bar/simple will change to /foo/simple'); + expect(warnSpy.calls.first().args[0]) + .toContain('relativeLinkResolution: \'legacy\' is deprecated'); }))); it('should ignore empty paths in corrected mode', @@ -5888,6 +5893,7 @@ describe('Integration', () => { it('should not ignore empty path when in legacy mode', fakeAsync(inject([Router], (router: Router) => { + const warnSpy = spyOn(console, 'warn'); router.relativeLinkResolution = 'legacy'; const fixture = createRoot(router, RootCmp); @@ -5899,6 +5905,10 @@ describe('Integration', () => { const link = fixture.nativeElement.querySelector('a'); expect(link.getAttribute('href')).toEqual('/lazy/foo/bar/simple'); + expect(warnSpy.calls.first().args[0]) + .toContain('/lazy/foo/bar/simple will change to /lazy/foo/simple'); + expect(warnSpy.calls.first().args[0]) + .toContain('relativeLinkResolution: \'legacy\' is deprecated'); }))); it('should ignore empty path when in corrected mode',