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
angular · Apr 11, 2022 · d180db1 · d180db1
1 parent f1630bb
commit d180db1
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 19 deletions.
diff --git 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.
+
 <a id="loadChildren"></a>
 
 ### loadChildren string syntax

diff --git 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 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.`);
+    }
+  }
 
-  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);
+  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
@@ -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<RouterStateSnapshot> {
@@ -159,24 +161,31 @@ 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) {
         return null;
       }
       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 ?? 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._segmentIndexShift ? s._segmentIndexShift : 0);
+    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
@@ -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<any>|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
@@ -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
@@ -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
@@ -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',