Skip to content

Commit

Permalink
Prevent recognize from poluting the shared ROUTE_INFOS map by condito…
Browse files Browse the repository at this point in the history
…nally using a local map for toReadOnlyRouteInfo. Always using a local map for toReadOnlyRouteInfo causes many tests to fail so apparently it is expected to polute state in many instances.
  • Loading branch information
chbonser committed Nov 28, 2023
1 parent dc948ab commit 95935a6
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 17 deletions.
34 changes: 25 additions & 9 deletions lib/router/route-info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,19 +63,30 @@ let ROUTE_INFOS = new WeakMap<RouteInfosKey, RouteInfo | RouteInfoWithAttributes
export function toReadOnlyRouteInfo<R extends Route>(
routeInfos: InternalRouteInfo<R>[],
queryParams: Dict<unknown> = {},
includeAttributes = false
options: {
includeAttributes?: boolean;
localizeMapUpdates?: boolean;
} = { includeAttributes: false, localizeMapUpdates: false }
): RouteInfoWithAttributes[] | RouteInfo[] {
const LOCAL_ROUTE_INFOS = new WeakMap<RouteInfosKey, RouteInfo | RouteInfoWithAttributes>();

return routeInfos.map((info, i) => {
let { name, params, paramNames, context, route } = info;
// SAFETY: This should be safe since it is just for use as a key
let key = (info as unknown) as RouteInfosKey;
if (ROUTE_INFOS.has(key) && includeAttributes) {
if (ROUTE_INFOS.has(key) && options.includeAttributes) {
let routeInfo = ROUTE_INFOS.get(key)!;
routeInfo = attachMetadata(route!, routeInfo);
let routeInfoWithAttribute = createRouteInfoWithAttributes(routeInfo, context);
ROUTE_INFOS.set(key, routeInfoWithAttribute);
LOCAL_ROUTE_INFOS.set(key, routeInfo);
if (!options.localizeMapUpdates) {
ROUTE_INFOS.set(key, routeInfoWithAttribute);
}
return routeInfoWithAttribute as RouteInfoWithAttributes;
}

const routeInfosRef = options.localizeMapUpdates ? LOCAL_ROUTE_INFOS : ROUTE_INFOS;

let routeInfo: RouteInfo = {
find(
predicate: (this: any, routeInfo: RouteInfo, i: number, arr?: RouteInfo[]) => boolean,
Expand All @@ -87,13 +98,13 @@ export function toReadOnlyRouteInfo<R extends Route>(
if (predicate.length === 3) {
arr = routeInfos.map(
// SAFETY: This should be safe since it is just for use as a key
(info) => ROUTE_INFOS.get((info as unknown) as RouteInfosKey)!
(info) => routeInfosRef.get((info as unknown) as RouteInfosKey)!
);
}

for (let i = 0; routeInfos.length > i; i++) {
// SAFETY: This should be safe since it is just for use as a key
publicInfo = ROUTE_INFOS.get((routeInfos[i] as unknown) as RouteInfosKey)!;
publicInfo = routeInfosRef.get((routeInfos[i] as unknown) as RouteInfosKey)!;
if (predicate.call(thisArg, publicInfo, i, arr)) {
return publicInfo;
}
Expand Down Expand Up @@ -122,7 +133,7 @@ export function toReadOnlyRouteInfo<R extends Route>(
}

// SAFETY: This should be safe since it is just for use as a key
return ROUTE_INFOS.get((parent as unknown) as RouteInfosKey)!;
return routeInfosRef.get((parent as unknown) as RouteInfosKey)!;
},

get child() {
Expand All @@ -133,7 +144,7 @@ export function toReadOnlyRouteInfo<R extends Route>(
}

// SAFETY: This should be safe since it is just for use as a key
return ROUTE_INFOS.get((child as unknown) as RouteInfosKey)!;
return routeInfosRef.get((child as unknown) as RouteInfosKey)!;
},

get localName() {
Expand All @@ -150,12 +161,17 @@ export function toReadOnlyRouteInfo<R extends Route>(
},
};

if (includeAttributes) {
if (options.includeAttributes) {
routeInfo = createRouteInfoWithAttributes(routeInfo, context);
}

// SAFETY: This should be safe since it is just for use as a key
ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);
LOCAL_ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);

if (!options.localizeMapUpdates) {
// SAFETY: This should be safe since it is just for use as a key
ROUTE_INFOS.set((info as unknown) as RouteInfosKey, routeInfo);
}

return routeInfo;
});
Expand Down
21 changes: 13 additions & 8 deletions lib/router/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,10 @@ export default abstract class Router<R extends Route> {
return newState;
}

let readonlyInfos = toReadOnlyRouteInfo(newState.routeInfos, newState.queryParams);
let readonlyInfos = toReadOnlyRouteInfo(newState.routeInfos, newState.queryParams, {
includeAttributes: false,
localizeMapUpdates: true,
});
return readonlyInfos[readonlyInfos.length - 1] as RouteInfo;
}

Expand All @@ -188,7 +191,10 @@ export default abstract class Router<R extends Route> {
let routeInfosWithAttributes = toReadOnlyRouteInfo(
newState!.routeInfos,
newTransition[QUERY_PARAMS_SYMBOL],
true
{
includeAttributes: true,
localizeMapUpdates: false,
}
) as RouteInfoWithAttributes[];
return routeInfosWithAttributes[routeInfosWithAttributes.length - 1];
});
Expand Down Expand Up @@ -773,11 +779,10 @@ export default abstract class Router<R extends Route> {

private fromInfos(newTransition: OpaqueTransition, oldRouteInfos: InternalRouteInfo<R>[]) {
if (newTransition !== undefined && oldRouteInfos.length > 0) {
let fromInfos = toReadOnlyRouteInfo(
oldRouteInfos,
Object.assign({}, this._lastQueryParams),
true
) as RouteInfoWithAttributes[];
let fromInfos = toReadOnlyRouteInfo(oldRouteInfos, Object.assign({}, this._lastQueryParams), {
includeAttributes: true,
localizeMapUpdates: false,
}) as RouteInfoWithAttributes[];
newTransition!.from = fromInfos[fromInfos.length - 1] || null;
}
}
Expand All @@ -791,7 +796,7 @@ export default abstract class Router<R extends Route> {
let toInfos = toReadOnlyRouteInfo(
newRouteInfos,
Object.assign({}, newTransition[QUERY_PARAMS_SYMBOL]),
includeAttributes
{ includeAttributes, localizeMapUpdates: false }
);
newTransition!.to = toInfos[toInfos.length - 1] || null;
}
Expand Down

0 comments on commit 95935a6

Please sign in to comment.