Visitar URL original
fix(router): do not copy URL-sized objects into every route snapshot · angular/angular@568d6a8 · GitHub
Skip to content

Commit 568d6a8

Browse files
SkyZeroZxatscott
authored andcommitted
fix(router): do not copy URL-sized objects into every route snapshot
An empty path route matches once for every outlet a URL names, so recognition copied the query params and the inherited params into as many snapshots as the URL asked for, at the width the URL chose. Share the frozen objects instead. Fixes #70932 (cherry picked from commit fac2f23)
1 parent 5df6094 commit 568d6a8

3 files changed

Lines changed: 67 additions & 8 deletions

File tree

‎packages/router/src/recognize.ts‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ export class Recognizer {
7272
private absoluteRedirectCount = 0;
7373
allowRedirects = true;
7474

75+
/** Shared by every snapshot: query params belong to the URL, not to any one route. */
76+
private queryParams: Params;
77+
7578
constructor(
7679
private injector: EnvironmentInjector,
7780
private configLoader: RouterConfigLoader,
@@ -83,6 +86,7 @@ export class Recognizer {
8386
private readonly abortSignal: AbortSignal,
8487
) {
8588
this.applyRedirects = new ApplyRedirects(this.urlSerializer, this.urlTree);
89+
this.queryParams = Object.freeze({...this.urlTree.queryParams});
8690
}
8791

8892
private noMatchError(e: NoMatch): RuntimeError<RuntimeErrorCode.NO_MATCH> {
@@ -123,7 +127,7 @@ export class Recognizer {
123127
const rootSnapshot = new ActivatedRouteSnapshot(
124128
[],
125129
Object.freeze({}),
126-
Object.freeze({...this.urlTree.queryParams}),
130+
this.queryParams,
127131
this.urlTree.fragment,
128132
Object.freeze({}),
129133
PRIMARY_OUTLET,
@@ -144,6 +148,7 @@ export class Recognizer {
144148
} catch (e: any) {
145149
if (e instanceof AbsoluteRedirect) {
146150
this.urlTree = e.urlTree;
151+
this.queryParams = Object.freeze({...this.urlTree.queryParams});
147152
return this.match(e.urlTree.root);
148153
}
149154
if (e instanceof NoMatch) {
@@ -384,7 +389,7 @@ export class Recognizer {
384389
const snapshot = new ActivatedRouteSnapshot(
385390
segments,
386391
parameters,
387-
Object.freeze({...this.urlTree.queryParams}),
392+
this.queryParams,
388393
this.urlTree.fragment,
389394
getData(route),
390395
getOutlet(route),
@@ -394,8 +399,8 @@ export class Recognizer {
394399
injector,
395400
);
396401
const inherited = getInherited(snapshot, parentRoute, this.paramsInheritanceStrategy);
397-
snapshot.params = Object.freeze(inherited.params);
398-
snapshot.data = Object.freeze(inherited.data);
402+
snapshot.params = inherited.params;
403+
snapshot.data = inherited.data;
399404
return snapshot;
400405
}
401406

‎packages/router/src/router_state.ts‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -325,8 +325,12 @@ export function getInherited(
325325
(!parent.component && !parent.routeConfig?.loadComponent))
326326
) {
327327
inherited = {
328-
params: {...parent.params, ...route.params},
329-
data: {...parent.data, ...route.data},
328+
// Reuse the parent's frozen params when the route adds none of its own.
329+
params:
330+
Object.keys(route.params).length === 0
331+
? parent.params
332+
: Object.freeze({...parent.params, ...route.params}),
333+
data: Object.freeze({...parent.data, ...route.data}),
330334
resolve: {
331335
// Snapshots are created with data inherited from parent and guards (i.e. canActivate) can
332336
// change data because it's not frozen...
@@ -345,8 +349,8 @@ export function getInherited(
345349
};
346350
} else {
347351
inherited = {
348-
params: {...route.params},
349-
data: {...route.data},
352+
params: Object.freeze({...route.params}),
353+
data: Object.freeze({...route.data}),
350354
resolve: {...route.data, ...(route._resolvedData ?? {})},
351355
};
352356
}

‎packages/router/test/recognize.spec.ts‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,8 +1144,58 @@ describe('recognize', () => {
11441144
await expectAsync(recognizer('foo/bar')).toBeRejected();
11451145
});
11461146
});
1147+
1148+
describe('snapshot creation', () => {
1149+
it('shares one queryParams object across every snapshot', async () => {
1150+
const s = await recognize(
1151+
[{path: 'a', children: [{path: '', children: [{path: '', component: ComponentA}]}]}],
1152+
'a/(x:/()//y:/()//z:/())?p=1&q=2',
1153+
);
1154+
const snapshots = collectSnapshots(s.root);
1155+
expect(snapshots.length).toBeGreaterThan(1);
1156+
for (const snapshot of snapshots) {
1157+
expect(snapshot.queryParams).toBe(s.root.queryParams);
1158+
}
1159+
expect(s.root.queryParams).toEqual({p: '1', q: '2'});
1160+
expect(Object.isFrozen(s.root.queryParams)).toBeTrue();
1161+
});
1162+
1163+
it('shares the parent params with a route that adds none of its own', async () => {
1164+
const s = await recognize(
1165+
[
1166+
{
1167+
path: 'a',
1168+
children: [
1169+
{path: '', component: ComponentA},
1170+
{path: '', outlet: 'modal', component: ComponentB},
1171+
],
1172+
},
1173+
],
1174+
'a;x=1/(modal:/())',
1175+
);
1176+
const a = s.root.firstChild!;
1177+
expect(a.params).toEqual({x: '1'});
1178+
// An empty path route matches once for every outlet the URL names.
1179+
expect(a.children.length).toBe(2);
1180+
for (const child of a.children) {
1181+
expect(child.params).toBe(a.params);
1182+
expect(Object.isFrozen(child.params)).toBeTrue();
1183+
}
1184+
});
1185+
});
11471186
});
11481187

1188+
function collectSnapshots(
1189+
snapshot: ActivatedRouteSnapshot,
1190+
into: ActivatedRouteSnapshot[] = [],
1191+
): ActivatedRouteSnapshot[] {
1192+
into.push(snapshot);
1193+
for (const child of snapshot.children) {
1194+
collectSnapshots(child, into);
1195+
}
1196+
return into;
1197+
}
1198+
11491199
async function recognize(
11501200
config: Routes,
11511201
url: string,

0 commit comments

Comments
 (0)