Merge pull request #336 from chbonser/recognize-qp-issue

Calling recognize should not affect the transition.from query params for subsequent transitions

lib/router/route-info.ts

@@ -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,
@@ -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;
           }
@@ -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() {
@@ -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() {
@@ -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;
   });

lib/router/router.ts

@@ -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;
   }
 
@@ -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];
     });
@@ -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;
     }
   }
@@ -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;
     }

tests/router_test.ts

@@ -1324,6 +1324,67 @@ scenarios.forEach(function (scenario) {
     });
   });
 
+  test('calling recognize should not affect the transition.from query params for subsequent transitions', function (assert) {
+    assert.expect(12);
+    map(assert, function (match) {
+      match('/').to('index');
+      match('/search').to('search');
+    });
+
+    routes = {
+      search: createHandler('search'),
+    };
+
+    let firstParam = false;
+    let secondParam = false;
+
+    router.routeWillChange = (transition: Transition) => {
+      if (secondParam) {
+        assert.deepEqual(transition.to!.queryParams, { term: 'c' }, 'going to next page with qps');
+        assert.deepEqual(
+          isPresent(transition.from) && transition.from!.queryParams,
+          { term: 'b' },
+          'has previous qps'
+        );
+      } else if (firstParam) {
+        assert.deepEqual(transition.to!.queryParams, { term: 'b' }, 'going to page with qps');
+        assert.deepEqual(
+          isPresent(transition.from) && transition.from!.queryParams,
+          {},
+          'from never has qps'
+        );
+      } else {
+        assert.equal(transition.from, null);
+        assert.deepEqual(transition.to!.queryParams, {});
+      }
+    };
+
+    router.routeDidChange = (transition: Transition) => {
+      if (secondParam) {
+        assert.deepEqual(transition.to!.queryParams, { term: 'c' });
+        assert.deepEqual(isPresent(transition.from) && transition.from!.queryParams, { term: 'b' });
+      } else if (firstParam) {
+        assert.deepEqual(transition.to!.queryParams, { term: 'b' });
+        assert.deepEqual(isPresent(transition.from) && transition.from!.queryParams, {});
+      } else {
+        assert.equal(transition.from, null);
+        assert.deepEqual(transition.to!.queryParams, {});
+      }
+    };
+
+    router
+      .transitionTo('/')
+      .then(() => {
+        firstParam = true;
+        return router.transitionTo('search', { queryParams: { term: 'b' } });
+      })
+      .then(() => {
+        secondParam = true;
+        router.recognize('/search?wat=foo');
+        return router.transitionTo({ queryParams: { term: 'c' } });
+      });
+  });
+
   test('redirects route events', function (assert) {
     assert.expect(19);
     map(assert, function (match) {