fix(normalizr): do not mutate cached journey on result-cache hit (#3925)

* chore: rebase PR #3925 onto latest master

* pkg: Bump peerdeps of @data-client/react to support 0.17 (#3927)

* pkg: Bump peerdeps of @data-client/react to support 0.17

* pkg: Update yarn.lock for peerdep bump

* perf(normalizr): store consumer-facing journey at write time (#3928)

Move the per-hit paths.slice(1) (and the hasStringDeps filter loop)
out of GlobalCache.getResults and into the cache write.

GlobalCache.paths() already produces the placeholder-free,
function-free shape every consumer needs. Hand it to
WeakDependencyMap.set as the journey, and the cache-hit branch can
return that array by reference - no per-hit allocation, no per-hit
typeof === 'function' walk.

Safety: paths is now a shared reference held by every subsequent
hit. The contract that consumers must not mutate it was established
by the journey-mutation fix (PR #3925) and is exercised by the
existing globalCache.test.ts regression test.

* fix: Scalar reversion

* test(core): add getResponseMeta-paths integration test (non-GC angle) (#3929)

* test(core): replace getResponseMeta-countRef with getResponseMeta-paths

The previous integration test poked GCPolicy['entityCount'] directly to
prove the journey-mutation bug. Replace it with a test that asserts the
public-API consequence the bug creates outside of GC: every subscriber
to the same endpoint must observe the same expiresAt from
Controller.getResponseMeta().

This is the property the bug actually broke for non-GC users: with the
buggy paths.shift(), entityExpiresAt(paths, …) iterates a
progressively-shorter list, dropping the entity with the earliest
expiry first. Subscriber 2 observes a too-late expiresAt; subscriber 3+
observe Infinity and never refetch. Fires under ImmortalGCPolicy too,
since entityExpiresAt is unconditional whenever the endpoint has no
top-level meta.expiresAt — typical for state populated via
controller.set(Entity, …), SSR hydration, or useQuery.

Verified the assertion fails on the buggy paths.shift() (m3.expiresAt
returns FOO_2_EXPIRY instead of FOO_1_EXPIRY) and passes on the fix.

* test(core): keep existing countRef integration test alongside paths test

Restore the GC-side getResponseMeta-countRef.ts integration test that
the prior commit replaced. The two tests cover the journey-mutation
bug from complementary angles:

- getResponseMeta-countRef.ts: GC consumer of paths (entityCount
  under-counting → premature reaping under default GCPolicy).
- getResponseMeta-paths.ts: non-GC consumer of paths
  (entityExpiresAt → suppressed entity-expiry refetch; fires under
  ImmortalGCPolicy too).

Both pass on the fix; both fail on the buggy paths.shift().

Author: ntucker

.changeset/fix-journey-mutation.md

@@ -0,0 +1,19 @@
+---
+'@data-client/normalizr': patch
+'@data-client/core': patch
+'@data-client/endpoint': patch
+'@data-client/rest': patch
+'@data-client/graphql': patch
+'@data-client/react': patch
+---
+
+Fix cached journey being mutated on repeated result-cache hits.
+
+`GlobalCache.getResults` called `paths.shift()` on a cache hit, mutating
+the `journey` array stored by reference on the `WeakDependencyMap` `Link`
+node. After the first hit stripped the placeholder input slot, every
+subsequent hit on the same cached entry would shift off a real
+`EntityPath`, progressively losing subscription entries. This could cause
+missed `countRef` tracking (premature GC of still-referenced entities)
+and incorrect `entityExpiresAt` calculations. The hit path now returns a
+non-mutating copy.

.changeset/peerdeps-017.md

@@ -0,0 +1,6 @@
+---
+'@data-client/img': patch
+'@data-client/test': patch
+---
+
+Bump `@data-client/react` peer dependency range to include `^0.17.0`.

packages/core/src/controller/__tests__/getResponseMeta-countRef.ts

@@ -0,0 +1,93 @@
+import { Endpoint, Entity } from '@data-client/endpoint';
+
+import { GCPolicy } from '../../state/GCPolicy';
+import { initialState } from '../../state/reducer/createReducer';
+import Controller from '../Controller';
+
+/** Integration test for the `paths` → `countRef` pipeline.
+ *
+ * React hooks (`useSuspense`, `useCache`, `useLive`, `useDLE`, `useQuery`)
+ * all call `controller.getResponseMeta(...)` during `useMemo` and then
+ * pass the returned `countRef` to `useEffect`. `useEffect` executes
+ * `countRef` once per hook instance — iterating the **captured** `paths`
+ * array to increment `GCPolicy.entityCount` per referenced entity. When
+ * the component unmounts (or `data` changes) the returned `decrement`
+ * closure iterates that same captured `paths` array to decrement.
+ *
+ * Before the fix, `globalCache.getResults` called `paths.shift()` on a
+ * result-cache hit, mutating the journey array stored by reference on
+ * `WeakDependencyMap.Link.journey`. Because `paths` aliases that stored
+ * journey, every successive hit dropped one more real `EntityPath` from
+ * the subscription list. The Nth component mount (for N ≥ 3, same
+ * endpoint + args, entities unchanged) would have its `countRef`
+ * iterate a progressively shorter list — skipping the first real
+ * entity, then the second, etc. — leaving `entityCount` under-counted
+ * for those entities and allowing GC to reap entities that mounted
+ * components still subscribe to.
+ */
+describe('Controller.getResponseMeta + GCPolicy.countRef integration', () => {
+  it('entityCount tracks all entities for each subscriber across repeated result-cache hits', () => {
+    class Foo extends Entity {
+      id = '';
+      pk() {
+        return this.id;
+      }
+    }
+    const ep = new Endpoint(() => Promise.resolve(), {
+      key: () => 'listFoo',
+      schema: [Foo],
+    });
+
+    const gcPolicy = new GCPolicy();
+    const controller = new Controller({ gcPolicy });
+    gcPolicy.init(controller);
+
+    const state = {
+      ...initialState,
+      entities: {
+        Foo: {
+          '1': { id: '1' },
+          '2': { id: '2' },
+        },
+      },
+      endpoints: {
+        [ep.key()]: ['1', '2'],
+      },
+    };
+
+    // Three successive `getResponseMeta` calls simulate three React
+    // components mounting with the same endpoint + args, where the store
+    // has not changed between renders. Call 1 is a result-cache miss;
+    // calls 2 and 3 are hits — each hit mutates the shared journey.
+    const m1 = controller.getResponseMeta(ep, state);
+    const m2 = controller.getResponseMeta(ep, state);
+    const m3 = controller.getResponseMeta(ep, state);
+
+    // Simulate `useEffect(countRef, [data])` firing for each mount.
+    // Pre-fix: m2.paths and m3.paths both aliased the shared journey,
+    // and by the time the countRefs run the journey has been shifted
+    // twice — so both iterate `[Foo|'2']` only, silently skipping
+    // Foo|'1'. Only m1 (miss path, fresh `paths`) counted Foo|'1'.
+    const dec1 = m1.countRef();
+    const dec2 = m2.countRef();
+    const dec3 = m3.countRef();
+
+    // Each mount must have incremented *both* entities.
+    // Pre-fix: Foo|'1' = 1 (only m1 counted it); Foo|'2' = 3.
+    expect(gcPolicy['entityCount'].get('Foo')?.get('1')).toBe(3);
+    expect(gcPolicy['entityCount'].get('Foo')?.get('2')).toBe(3);
+
+    // Simulate all three components unmounting (or `data` changing).
+    // Pre-fix: only dec1 would decrement Foo|'1' (taking it from 1 to
+    // 0) — observable as premature GC-eligibility of Foo|'1' while
+    // the other two subscribers still depended on it.
+    dec1();
+    dec2();
+    dec3();
+
+    expect(gcPolicy['entityCount'].get('Foo')?.get('1')).toBeUndefined();
+    expect(gcPolicy['entityCount'].get('Foo')?.get('2')).toBeUndefined();
+
+    gcPolicy.cleanup();
+  });
+});

packages/core/src/controller/__tests__/getResponseMeta-paths.ts

@@ -0,0 +1,88 @@
+import { Endpoint, Entity } from '@data-client/endpoint';
+
+import { initialState } from '../../state/reducer/createReducer';
+import Controller from '../Controller';
+
+/** Integration test: `Controller.getResponseMeta()` is the seam every read
+ * hook (`useSuspense`, `useCache`, `useFetch`, `useDLE`, `useLive`) calls.
+ * Multiple subscribers to the same endpoint + args + entities must observe
+ * the same `expiresAt`, otherwise late subscribers silently skip the
+ * entity-expiry refetch in `useSuspense`'s gate
+ * `if (Date.now() <= expiresAt && !forceFetch) return;`.
+ *
+ * The bug fixed by this PR was a `paths.shift()` mutation on the journey
+ * stored by reference on `WeakDependencyMap.Link.journey`. Each successive
+ * cache hit returned a journey one entry shorter than the last. The first
+ * entity dropped is the one with the *earliest* expiry, so subscriber 2's
+ * `entityExpiresAt(paths, …)` skipped it and computed a too-late
+ * `expiresAt`; subscriber 3 saw an empty array and got `Infinity`.
+ *
+ * The path is GC-independent: it fires under `ImmortalGCPolicy` too, since
+ * `entityExpiresAt` is unconditional whenever the endpoint has no
+ * top-level `meta.expiresAt` (the common case for state populated via
+ * `controller.set(Entity, …)`, SSR hydration, or `useQuery`).
+ */
+describe('Controller.getResponseMeta repeated calls', () => {
+  it('returns identical expiresAt across subscribers (journey is not consumed by reads)', () => {
+    class Foo extends Entity {
+      id = '';
+      pk() {
+        return this.id;
+      }
+    }
+    const ep = new Endpoint(() => Promise.resolve(), {
+      key: () => 'listFoo',
+      schema: [Foo],
+    });
+
+    const controller = new Controller({});
+
+    // Endpoint slot exists (so we hit the `denormalize` path) but the
+    // endpoint itself has no top-level `meta.expiresAt` yet — mirrors
+    // apps that hydrate from SSR or use `controller.set(Entity, …)`
+    // without going through an endpoint fetch. In that shape,
+    // `Controller.getResponseMeta()` falls back to walking per-entity
+    // meta via `entityExpiresAt(paths, …)`. Foo|'1' has the earliest
+    // expiry — the first journey entry the buggy `paths.shift()` would
+    // drop.
+    const FOO_1_EXPIRY = 100;
+    const FOO_2_EXPIRY = 1_000_000;
+    const state = {
+      ...initialState,
+      entities: {
+        Foo: {
+          '1': { id: '1' },
+          '2': { id: '2' },
+        },
+      },
+      entitiesMeta: {
+        Foo: {
+          '1': { date: 0, fetchedAt: 0, expiresAt: FOO_1_EXPIRY },
+          '2': { date: 0, fetchedAt: 0, expiresAt: FOO_2_EXPIRY },
+        },
+      },
+      endpoints: {
+        [ep.key()]: ['1', '2'],
+      },
+    };
+
+    // Three successive calls simulate three React components mounting
+    // with the same endpoint + args. Call 1 is a result-cache miss;
+    // calls 2 and 3 are hits.
+    const m1 = controller.getResponseMeta(ep, state);
+    const m2 = controller.getResponseMeta(ep, state);
+    const m3 = controller.getResponseMeta(ep, state);
+
+    // Subscriber 1 picks the earliest entity expiry. Subscribers 2+ must
+    // pick the same one — pre-fix they observed FOO_2_EXPIRY (m2) and
+    // Infinity (m3), silently suppressing entity-expiry refetch.
+    expect(m1.expiresAt).toBe(FOO_1_EXPIRY);
+    expect(m2.expiresAt).toBe(m1.expiresAt);
+    expect(m3.expiresAt).toBe(m1.expiresAt);
+
+    // Cached data must remain referentially equal across hits — the
+    // memo contract `useSuspense` etc. rely on for skip-rerender.
+    expect(m2.data).toBe(m1.data);
+    expect(m3.data).toBe(m1.data);
+  });
+});

packages/img/package.json

@@ -78,7 +78,7 @@
     "@data-client/endpoint": "workspace:^"
   },
   "peerDependencies": {
-    "@data-client/react": "^0.1.0 || ^0.2.0 || ^0.3.0 || ^0.4.0 || ^0.5.0 || ^0.7.0 || ^0.8.0 || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0",
+    "@data-client/react": "^0.1.0 || ^0.2.0 || ^0.3.0 || ^0.4.0 || ^0.5.0 || ^0.7.0 || ^0.8.0 || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0 || ^0.17.0",
     "@types/react": "^16.14.0 || ^17.0.0 || ^18.0.0-0 || ^19.0.0",
     "react": "^16.14.0 || ^17.0.0 || ^18.0.0-0 || ^19.0.0"
   },

packages/normalizr/src/__tests__/globalCache.test.ts

@@ -121,6 +121,47 @@ describe('GlobalCache', () => {
       expect(r2.paths).toEqual(r1.paths);
     });
 
+    it('does not mutate cached journey across repeated result-cache hits', () => {
+      // Regression: on a resultCache hit, `getResults` must strip the
+      // placeholder input slot from the returned paths WITHOUT mutating the
+      // journey array stored on the WeakDependencyMap `Link` node. A mutating
+      // `paths.shift()` progressively removes real entity paths from the
+      // stored journey on each successive hit, corrupting the subscription
+      // list (missed countRef, incorrect entityExpiresAt).
+      const entities = {
+        Foo: { '1': { id: '1' }, '2': { id: '2' } },
+      };
+      const { getEntity, getCache, resultCache } = makeDeps(entities);
+      const input = [{ id: '1' }, { id: '2' }];
+
+      const frame1 = new GlobalCache(getEntity, getCache, resultCache, []);
+      const r1 = frame1.getResults(input, true, () => {
+        frame1.getEntity('1', Foo, entities.Foo['1'], m =>
+          m.set('1', { ...entities.Foo['1'] }),
+        );
+        frame1.getEntity('2', Foo, entities.Foo['2'], m =>
+          m.set('2', { ...entities.Foo['2'] }),
+        );
+        return [{ id: '1' }, { id: '2' }];
+      });
+      expect(r1.paths).toEqual([
+        { key: 'Foo', pk: '1' },
+        { key: 'Foo', pk: '2' },
+      ]);
+
+      // Repeated hits must each return the full set of entity paths.
+      for (let n = 0; n < 3; n++) {
+        const frame = new GlobalCache(getEntity, getCache, resultCache, []);
+        const { paths } = frame.getResults(input, true, () => {
+          throw new Error('resultCache must hit');
+        });
+        expect(paths).toEqual([
+          { key: 'Foo', pk: '1' },
+          { key: 'Foo', pk: '2' },
+        ]);
+      }
+    });
+
     it('cache hit across frames also works when argsKey was used (paths filtered)', () => {
       // Once the resultCache has stored any function-typed dep, future hits
       // must strip them when returning paths. Ensures the on-hit filter at

packages/normalizr/src/memo/WeakDependencyMap.ts

@@ -69,7 +69,18 @@ export default class WeakDependencyMap<
     return [curLink.value, curLink.journey] as readonly [V, Path[]];
   }
 
-  set(dependencies: Dep<Path, K>[], value: V, args: readonly any[] = []) {
+  set(
+    dependencies: Dep<Path, K>[],
+    value: V,
+    args: readonly any[] = [],
+    /** Optional consumer-facing journey returned to `get()` callers verbatim.
+     * Defaults to `dependencies.map(d => d.path)`. Pass an explicit array to
+     * skip the per-write `.map(...)` and (more importantly) to skip per-hit
+     * post-processing — see `GlobalCache.getResults` for the read-side
+     * payoff. The array becomes a shared reference held by every subsequent
+     * cache hit; callers MUST NOT mutate it. */
+    journey?: Path[],
+  ) {
     if (dependencies.length < 1) throw new KeySize();
     let curLink: Link<Path, K, V> = this as any;
     for (const dep of dependencies) {
@@ -98,7 +109,7 @@ export default class WeakDependencyMap<
     curLink.nextPath = undefined;
     curLink.value = value;
     // we could recompute this on get, but it would have a cost and we optimize for `get`
-    curLink.journey = dependencies.map(d => d.path) as Path[];
+    curLink.journey = journey ?? (dependencies.map(d => d.path) as Path[]);
   }
 
   /** True once any `argsKey`-style dep has been written. Consumers can use

packages/normalizr/src/memo/globalCache.ts

@@ -173,24 +173,20 @@ export default class GlobalCache implements Cache {
 
     if (paths === undefined) {
       data = computeValue();
-      // we want to do this before we fill our 'input' entry
+      // build the consumer-facing subscription list once, here.
+      // `paths()` already excludes the input placeholder slot and (when
+      // `_hasArgsKey`) function-typed `argsKey` deps, so it's exactly the
+      // shape every hit-branch consumer needs. We hand it to the cache as
+      // the journey so subsequent hits return it by reference — no per-hit
+      // `slice(1)` or filter pass required.
       paths = this.paths();
-      // fill pre-allocated slot 0 with the input reference
+      // fill pre-allocated slot 0 with the input reference (chain key only)
       this.dependencies[0] = { path: { key: '', pk: '' }, entity: input };
-      this._resultCache.set(this.dependencies, data, this._args);
-    } else {
-      paths.shift();
-      // strip any function-typed (`argsKey`) paths — not subscribable entities.
-      // Only possible when the result cache has ever stored such a dep.
-      if (this._resultCache.hasStringDeps) {
-        for (let i = 0; i < paths.length; i++) {
-          if (typeof paths[i] === 'function') {
-            paths = paths.filter(p => typeof p !== 'function') as EntityPath[];
-            break;
-          }
-        }
-      }
+      this._resultCache.set(this.dependencies, data, this._args, paths);
     }
+    // hit branch: `paths` aliases the stored Link.journey — return as-is.
+    // Callers must not mutate it (entityExpiresAt and createCountRef both
+    // read-only). The journey-mutation regression test guards this contract.
     return { data, paths };
   }
 

packages/test/package.json

@@ -120,7 +120,7 @@
     "@testing-library/react": "^16.0.0"
   },
   "peerDependencies": {
-    "@data-client/react": "^0.12.15 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0",
+    "@data-client/react": "^0.12.15 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0 || ^0.17.0",
     "@testing-library/react-hooks": "^8.0.0",
     "@testing-library/react-native": "^13.0.0",
     "@types/react": "^16.14.0 || ^17.0.0 || ^18.0.0 || ^19.0.0-0 || ^19.0.0",

yarn.lock

@@ -3262,7 +3262,7 @@ __metadata:
     react-dom: "npm:^19.2.1"
     rollup-plugins: "workspace:*"
   peerDependencies:
-    "@data-client/react": ^0.1.0 || ^0.2.0 || ^0.3.0 || ^0.4.0 || ^0.5.0 || ^0.7.0 || ^0.8.0 || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0
+    "@data-client/react": ^0.1.0 || ^0.2.0 || ^0.3.0 || ^0.4.0 || ^0.5.0 || ^0.7.0 || ^0.8.0 || ^0.9.0 || ^0.10.0 || ^0.11.0 || ^0.12.0 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0 || ^0.17.0
     "@types/react": ^16.14.0 || ^17.0.0 || ^18.0.0-0 || ^19.0.0
     react: ^16.14.0 || ^17.0.0 || ^18.0.0-0 || ^19.0.0
   peerDependenciesMeta:
@@ -3369,7 +3369,7 @@ __metadata:
     react-test-renderer: "npm:*"
     rollup-plugins: "workspace:*"
   peerDependencies:
-    "@data-client/react": ^0.12.15 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0
+    "@data-client/react": ^0.12.15 || ^0.13.0 || ^0.14.0 || ^0.15.0 || ^0.16.0 || ^0.17.0
     "@testing-library/react-hooks": ^8.0.0
     "@testing-library/react-native": ^13.0.0
     "@types/react": ^16.14.0 || ^17.0.0 || ^18.0.0 || ^19.0.0-0 || ^19.0.0