From 95935a6a61b7aa109c52e4a0642e7adccf5afb49 Mon Sep 17 00:00:00 2001 From: Chris Bonser <1022582+chbonser@users.noreply.github.com> Date: Tue, 28 Nov 2023 14:25:56 -0600 Subject: [PATCH] Prevent recognize from poluting the shared ROUTE_INFOS map by conditonally 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. --- lib/router/route-info.ts | 34 +++++++++++++++++++++++++--------- lib/router/router.ts | 21 +++++++++++++-------- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/lib/router/route-info.ts b/lib/router/route-info.ts index a7dcd80..9938700 100644 --- a/lib/router/route-info.ts +++ b/lib/router/route-info.ts @@ -63,19 +63,30 @@ let ROUTE_INFOS = new WeakMap( routeInfos: InternalRouteInfo[], queryParams: Dict = {}, - includeAttributes = false + options: { + includeAttributes?: boolean; + localizeMapUpdates?: boolean; + } = { includeAttributes: false, localizeMapUpdates: false } ): RouteInfoWithAttributes[] | RouteInfo[] { + const LOCAL_ROUTE_INFOS = new WeakMap(); + 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, @@ -87,13 +98,13 @@ export function toReadOnlyRouteInfo( 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; } @@ -122,7 +133,7 @@ export function toReadOnlyRouteInfo( } // 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() { @@ -133,7 +144,7 @@ export function toReadOnlyRouteInfo( } // 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() { @@ -150,12 +161,17 @@ export function toReadOnlyRouteInfo( }, }; - 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; }); diff --git a/lib/router/router.ts b/lib/router/router.ts index 06979a8..cd73f4c 100644 --- a/lib/router/router.ts +++ b/lib/router/router.ts @@ -171,7 +171,10 @@ export default abstract class Router { 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; } @@ -188,7 +191,10 @@ export default abstract class Router { let routeInfosWithAttributes = toReadOnlyRouteInfo( newState!.routeInfos, newTransition[QUERY_PARAMS_SYMBOL], - true + { + includeAttributes: true, + localizeMapUpdates: false, + } ) as RouteInfoWithAttributes[]; return routeInfosWithAttributes[routeInfosWithAttributes.length - 1]; }); @@ -773,11 +779,10 @@ export default abstract class Router { private fromInfos(newTransition: OpaqueTransition, oldRouteInfos: InternalRouteInfo[]) { 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; } } @@ -791,7 +796,7 @@ export default abstract class Router { let toInfos = toReadOnlyRouteInfo( newRouteInfos, Object.assign({}, newTransition[QUERY_PARAMS_SYMBOL]), - includeAttributes + { includeAttributes, localizeMapUpdates: false } ); newTransition!.to = toInfos[toInfos.length - 1] || null; }