Skip to content

Commit 47e7a28

Browse files
atscottthePunderWoman
authored andcommitted
refactor(router): Add warning for relativeLinkResolution: 'legacy' (angular#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 angular#45523
1 parent 598b759 commit 47e7a28

File tree

7 files changed

+89
-19
lines changed

7 files changed

+89
-19
lines changed

aio/content/guide/deprecations.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,8 @@ The `relativeLinkResolution` option is deprecated and being removed.
317317
In version 11, the default behavior was changed to the correct one.
318318
After `relativeLinkResolution` is removed, the correct behavior is always used without an option to use the broken behavior.
319319

320+
A dev mode warning was added in v14 to warn if a created `UrlTree` relies on the `relativeLinkResolution: 'legacy'` option.
321+
320322
<a id="loadChildren"></a>
321323

322324
### loadChildren string syntax

packages/router/src/create_url_tree.ts

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,13 +24,34 @@ export function createUrlTree(
2424
return tree(urlTree.root, urlTree.root, new UrlSegmentGroup([], {}), queryParams, fragment);
2525
}
2626

27-
const startingPosition = findStartingPosition(nav, urlTree, route);
27+
function createTreeUsingPathIndex(lastPathIndex: number) {
28+
const startingPosition =
29+
findStartingPosition(nav, urlTree, route.snapshot?._urlSegment, lastPathIndex);
30+
31+
const segmentGroup = startingPosition.processChildren ?
32+
updateSegmentGroupChildren(
33+
startingPosition.segmentGroup, startingPosition.index, nav.commands) :
34+
updateSegmentGroup(startingPosition.segmentGroup, startingPosition.index, nav.commands);
35+
return tree(urlTree.root, startingPosition.segmentGroup, segmentGroup, queryParams, fragment);
36+
}
37+
// Note: The types should disallow `snapshot` from being `undefined` but due to test mocks, this
38+
// may be the case. Since we try to access it at an earlier point before the refactor to add the
39+
// warning for `relativeLinkResolution: 'legacy'`, this may cause failures in tests where it
40+
// didn't before.
41+
const result = createTreeUsingPathIndex(route.snapshot?._lastPathIndex);
42+
43+
// Check if application is relying on `relativeLinkResolution: 'legacy'`
44+
if (typeof ngDevMode === 'undefined' || !!ngDevMode) {
45+
const correctedResult = createTreeUsingPathIndex(route.snapshot?._correctedLastPathIndex);
46+
if (correctedResult.toString() !== result.toString()) {
47+
console.warn(
48+
`relativeLinkResolution: 'legacy' is deprecated and will be removed in a future version of Angular. The link to ${
49+
result.toString()} will change to ${
50+
correctedResult.toString()} if the code is not updated before then.`);
51+
}
52+
}
2853

29-
const segmentGroup = startingPosition.processChildren ?
30-
updateSegmentGroupChildren(
31-
startingPosition.segmentGroup, startingPosition.index, nav.commands) :
32-
updateSegmentGroup(startingPosition.segmentGroup, startingPosition.index, nav.commands);
33-
return tree(urlTree.root, startingPosition.segmentGroup, segmentGroup, queryParams, fragment);
54+
return result;
3455
}
3556

3657
function isMatrixParams(command: any): boolean {
@@ -151,13 +172,14 @@ class Position {
151172
}
152173
}
153174

154-
function findStartingPosition(nav: Navigation, tree: UrlTree, route: ActivatedRoute): Position {
175+
function findStartingPosition(
176+
nav: Navigation, tree: UrlTree, segmentGroup: UrlSegmentGroup,
177+
lastPathIndex: number): Position {
155178
if (nav.isAbsolute) {
156179
return new Position(tree.root, true, 0);
157180
}
158181

159-
if (route.snapshot._lastPathIndex === -1) {
160-
const segmentGroup = route.snapshot._urlSegment;
182+
if (lastPathIndex === -1) {
161183
// Pathless ActivatedRoute has _lastPathIndex === -1 but should not process children
162184
// see issue #26224, #13011, #35687
163185
// However, if the ActivatedRoute is the root we should process children like above.
@@ -166,9 +188,8 @@ function findStartingPosition(nav: Navigation, tree: UrlTree, route: ActivatedRo
166188
}
167189

168190
const modifier = isMatrixParams(nav.commands[0]) ? 0 : 1;
169-
const index = route.snapshot._lastPathIndex + modifier;
170-
return createPositionApplyingDoubleDots(
171-
route.snapshot._urlSegment, index, nav.numberOfDoubleDots);
191+
const index = lastPathIndex + modifier;
192+
return createPositionApplyingDoubleDots(segmentGroup, index, nav.numberOfDoubleDots);
172193
}
173194

174195
function createPositionApplyingDoubleDots(

packages/router/src/recognize.ts

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {getOutlet, sortByMatchingOutlets} from './utils/config';
1818
import {isImmediateMatch, match, noLeftoversInUrl, split} from './utils/config_matching';
1919
import {TreeNode} from './utils/tree';
2020

21+
const NG_DEV_MODE = typeof ngDevMode === 'undefined' || !!ngDevMode;
22+
2123
class NoMatch {}
2224

2325
function newObservableError(e: unknown): Observable<RouterStateSnapshot> {
@@ -159,24 +161,31 @@ export class Recognizer {
159161

160162
if (route.path === '**') {
161163
const params = segments.length > 0 ? last(segments)!.parameters : {};
164+
const pathIndexShift = getPathIndexShift(rawSegment) + segments.length;
162165
snapshot = new ActivatedRouteSnapshot(
163166
segments, params, Object.freeze({...this.urlTree.queryParams}), this.urlTree.fragment,
164167
getData(route), getOutlet(route), route.component!, route,
165-
getSourceSegmentGroup(rawSegment), getPathIndexShift(rawSegment) + segments.length,
166-
getResolve(route));
168+
getSourceSegmentGroup(rawSegment), pathIndexShift, getResolve(route),
169+
// NG_DEV_MODE is used to prevent the getCorrectedPathIndexShift function from affecting
170+
// production bundle size. This value is intended only to surface a warning to users
171+
// depending on `relativeLinkResolution: 'legacy'` in dev mode.
172+
(NG_DEV_MODE ? getCorrectedPathIndexShift(rawSegment) + segments.length :
173+
pathIndexShift));
167174
} else {
168175
const result = match(rawSegment, route, segments);
169176
if (!result.matched) {
170177
return null;
171178
}
172179
consumedSegments = result.consumedSegments;
173180
remainingSegments = result.remainingSegments;
181+
const pathIndexShift = getPathIndexShift(rawSegment) + consumedSegments.length;
174182

175183
snapshot = new ActivatedRouteSnapshot(
176184
consumedSegments, result.parameters, Object.freeze({...this.urlTree.queryParams}),
177185
this.urlTree.fragment, getData(route), getOutlet(route), route.component!, route,
178-
getSourceSegmentGroup(rawSegment),
179-
getPathIndexShift(rawSegment) + consumedSegments.length, getResolve(route));
186+
getSourceSegmentGroup(rawSegment), pathIndexShift, getResolve(route),
187+
(NG_DEV_MODE ? getCorrectedPathIndexShift(rawSegment) + consumedSegments.length :
188+
pathIndexShift));
180189
}
181190

182191
const childConfig: Route[] = getChildConfig(route);
@@ -303,10 +312,20 @@ function getSourceSegmentGroup(segmentGroup: UrlSegmentGroup): UrlSegmentGroup {
303312

304313
function getPathIndexShift(segmentGroup: UrlSegmentGroup): number {
305314
let s = segmentGroup;
306-
let res = (s._segmentIndexShift ? s._segmentIndexShift : 0);
315+
let res = s._segmentIndexShift ?? 0;
316+
while (s._sourceSegment) {
317+
s = s._sourceSegment;
318+
res += s._segmentIndexShift ?? 0;
319+
}
320+
return res - 1;
321+
}
322+
323+
function getCorrectedPathIndexShift(segmentGroup: UrlSegmentGroup): number {
324+
let s = segmentGroup;
325+
let res = s._segmentIndexShiftCorrected ?? s._segmentIndexShift ?? 0;
307326
while (s._sourceSegment) {
308327
s = s._sourceSegment;
309-
res += (s._segmentIndexShift ? s._segmentIndexShift : 0);
328+
res += s._segmentIndexShiftCorrected ?? s._segmentIndexShift ?? 0;
310329
}
311330
return res - 1;
312331
}

packages/router/src/router_state.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,13 @@ export class ActivatedRouteSnapshot {
284284
_urlSegment: UrlSegmentGroup;
285285
/** @internal */
286286
_lastPathIndex: number;
287+
/**
288+
* @internal
289+
*
290+
* Used only in dev mode to detect if application relies on `relativeLinkResolution: 'legacy'`
291+
* Should be removed in v16.
292+
*/
293+
_correctedLastPathIndex: number;
287294
/** @internal */
288295
_resolve: ResolveData;
289296
/** @internal */
@@ -333,10 +340,11 @@ export class ActivatedRouteSnapshot {
333340
public outlet: string,
334341
/** The component of the route */
335342
public component: Type<any>|string|null, routeConfig: Route|null, urlSegment: UrlSegmentGroup,
336-
lastPathIndex: number, resolve: ResolveData) {
343+
lastPathIndex: number, resolve: ResolveData, correctedLastPathIndex?: number) {
337344
this.routeConfig = routeConfig;
338345
this._urlSegment = urlSegment;
339346
this._lastPathIndex = lastPathIndex;
347+
this._correctedLastPathIndex = correctedLastPathIndex ?? lastPathIndex;
340348
this._resolve = resolve;
341349
}
342350

packages/router/src/url_tree.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,13 @@ export class UrlSegmentGroup {
227227
_sourceSegment?: UrlSegmentGroup;
228228
/** @internal */
229229
_segmentIndexShift?: number;
230+
/**
231+
* @internal
232+
*
233+
* Used only in dev mode to detect if application relies on `relativeLinkResolution: 'legacy'`
234+
* Should be removed in when `relativeLinkResolution` is removed.
235+
*/
236+
_segmentIndexShiftCorrected?: number;
230237
/** The parent node in the url tree */
231238
parent: UrlSegmentGroup|null = null;
232239

packages/router/src/utils/config_matching.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,9 @@ function addEmptyPathsToChildrenIfNeeded(
111111
s._sourceSegment = segmentGroup;
112112
if (relativeLinkResolution === 'legacy') {
113113
s._segmentIndexShift = segmentGroup.segments.length;
114+
if (typeof ngDevMode === 'undefined' || !!ngDevMode) {
115+
s._segmentIndexShiftCorrected = consumedSegments.length;
116+
}
114117
} else {
115118
s._segmentIndexShift = consumedSegments.length;
116119
}

packages/router/test/integration.spec.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,7 @@ describe('Integration', () => {
8787

8888
it('should not ignore empty paths in legacy mode',
8989
fakeAsync(inject([Router], (router: Router) => {
90+
const warnSpy = spyOn(console, 'warn');
9091
router.relativeLinkResolution = 'legacy';
9192

9293
const fixture = createRoot(router, RootCmp);
@@ -96,6 +97,10 @@ describe('Integration', () => {
9697

9798
const link = fixture.nativeElement.querySelector('a');
9899
expect(link.getAttribute('href')).toEqual('/foo/bar/simple');
100+
expect(warnSpy.calls.first().args[0])
101+
.toContain('/foo/bar/simple will change to /foo/simple');
102+
expect(warnSpy.calls.first().args[0])
103+
.toContain('relativeLinkResolution: \'legacy\' is deprecated');
99104
})));
100105

101106
it('should ignore empty paths in corrected mode',
@@ -5888,6 +5893,7 @@ describe('Integration', () => {
58885893

58895894
it('should not ignore empty path when in legacy mode',
58905895
fakeAsync(inject([Router], (router: Router) => {
5896+
const warnSpy = spyOn(console, 'warn');
58915897
router.relativeLinkResolution = 'legacy';
58925898

58935899
const fixture = createRoot(router, RootCmp);
@@ -5899,6 +5905,10 @@ describe('Integration', () => {
58995905

59005906
const link = fixture.nativeElement.querySelector('a');
59015907
expect(link.getAttribute('href')).toEqual('/lazy/foo/bar/simple');
5908+
expect(warnSpy.calls.first().args[0])
5909+
.toContain('/lazy/foo/bar/simple will change to /lazy/foo/simple');
5910+
expect(warnSpy.calls.first().args[0])
5911+
.toContain('relativeLinkResolution: \'legacy\' is deprecated');
59025912
})));
59035913

59045914
it('should ignore empty path when in corrected mode',

0 commit comments

Comments
 (0)