Merge pull request #10 from effector/fix-shallow-routing-races

Fix shallow routing races

Author: AlexandrHoroshih

src/get-scope.test.ts

@@ -88,4 +88,32 @@ describe("getClientScope", () => {
     expect(clientScopeOne.getState(longUpFx.inFlight)).toEqual(0);
     expect(clientScopeOne.getState($specialData)).toEqual(getFixedDate());
   });
+  test("shallow navigation to same page", async () => {
+    const serverScope = fork();
+
+    await allSettled(up, { scope: serverScope });
+    await allSettled(up, { scope: serverScope });
+    await allSettled(up, { scope: serverScope });
+
+    const values = serialize(serverScope);
+
+    const clientScopeOne = getClientScope(values);
+
+    expect(clientScopeOne.getState($count)).toEqual(3);
+
+    await allSettled(up, { scope: clientScopeOne });
+
+    expect(clientScopeOne.getState($count)).toEqual(4);
+
+    // This imitates shallow navigation to same page, e.g. with different query params
+    //
+    // Next.js will reuse the same pageProps instance in this case
+    // which will basically override current page state with initial one
+    //
+    // So we need to basically just ignore it, because
+    // we already have the latest state in the client scope
+    const clientScopeTwo = getClientScope(values);
+
+    expect(clientScopeTwo.getState($count)).toEqual(4);
+  });
 });

src/get-scope.ts

@@ -16,13 +16,31 @@ function getServerScope(values?: Values) {
  * This temporary solution on hacks allows us to solve the pain of library users when working with Next.js, as well as gather much more information to develop a better API.
  */
 const _currentScope: Scope = fork();
+let prevValues: Values;
 /**
  * @private
  *
  * exported for tests only
  */
 export function getClientScope(values?: Values) {
-  if (!values) return _currentScope;
+  if (
+    !values ||
+    /**
+     * This is a hack to handle edge cases with shallow navigation
+     *
+     * In this case Next.js will basically re-use old pageProps,
+     * but we already have latest state in the client scope
+     *
+     * So this update is just skipped
+     */
+    values === prevValues
+  )
+    return _currentScope;
+
+  /**
+   * Saving previous values to handle edge cases with shallow navigation
+   */
+  prevValues = values;
 
   HACK_injectValues(_currentScope, values);
   HACK_updateScopeRefs(_currentScope, values);
@@ -39,23 +57,26 @@ function HACK_updateScopeRefs(scope: Scope, values: Values) {
   // @ts-expect-error
   for (const id in scope.reg) {
     // @ts-expect-error
-    const ref = scope.reg[id];
-    if (!ref.meta || (!ref.meta?.named && ref.meta?.derived)) {
-      /**
-       * Force recalculation of derived values
-       */
+    if (Object.hasOwnProperty.call(scope.reg, id)) {
       // @ts-expect-error
-      delete scope.reg[id];
-    } else {
-      /**
-       * Update non-derived values
-       */
-      const sid = ref?.meta?.sid;
-      if (sid && sid in values) {
-        const serialize = ref?.meta?.serialize;
-        const read =
-          serialize && serialize !== "ingore" ? serialize?.read : null;
-        ref.current = read ? read(values[sid]) : values[sid];
+      const ref = scope.reg[id];
+      if (!ref.meta || (!ref.meta?.named && ref.meta?.derived)) {
+        /**
+         * Force recalculation of derived values
+         */
+        // @ts-expect-error
+        delete scope.reg[id];
+      } else {
+        /**
+         * Update non-derived values
+         */
+        const sid = ref?.meta?.sid;
+        if (sid && sid in values) {
+          const serialize = ref?.meta?.serialize;
+          const read =
+            serialize && serialize !== "ingore" ? serialize?.read : null;
+          ref.current = read ? read(values[sid]) : values[sid];
+        }
       }
     }
   }