fix(normalizr): Track INVALID and null nested entities as memo dependencies

.changeset/memo-invalid-null-deps.md

@@ -0,0 +1,10 @@
+---
+'@data-client/normalizr': patch
+'@data-client/core': patch
+'@data-client/react': patch
+'@data-client/vue': patch
+---
+
+Fix a cached parent showing a stale nested entity after it becomes valid
+
+When a nested entity was `INVALID` (for example, it failed validation) or `null` in the store, the parent it belongs to kept returning the old result even after that entity was valid again. Parents now update as soon as the nested entity does.

.claude/rules/memo-invariants.md

@@ -0,0 +1,13 @@
+---
+paths:
+  - "packages/normalizr/src/memo/**"
+  - "packages/normalizr/src/denormalize/**"
+  - "packages/normalizr/src/schemas/**"
+  - "packages/endpoint/src/schemas/**"
+---
+<!-- Generated from .cursor/rules/memo-invariants.mdc by `yarn build:agent-rules`. Edit the source. -->
+
+
+# Memo invariants
+
+Before changing memoization, or a schema's `denormalize`, read "Memo invariants" in `packages/normalizr/AGENTS.md`.

.cursor/rules/memo-invariants.mdc

@@ -0,0 +1,9 @@
+---
+description: Memoization invariants for normalizr's MemoCache and WeakDependencyMap
+globs: packages/normalizr/src/memo/**, packages/normalizr/src/denormalize/**, packages/normalizr/src/schemas/**, packages/endpoint/src/schemas/**
+alwaysApply: false
+---
+
+# Memo invariants
+
+Before changing memoization, or a schema's `denormalize`, read "Memo invariants" in `packages/normalizr/AGENTS.md`.

examples/benchmark/README.md

@@ -21,6 +21,7 @@ yarn workspace example-benchmark start [suite-name] [filter]
 ```
 
 Both arguments are optional:
+
 - **No arguments**: runs `normalizr` + `core` suites with all benchmarks
 - **Suite only**: `yarn start normalizr` runs all benchmarks in that suite
 - **Suite + filter**: `yarn start normalizr denormalize` runs only benchmarks containing "denormalize"
@@ -35,6 +36,7 @@ Both arguments are optional:
 - `entity` - benchmarks various entity-specific operations
 - `core` - benchmarks entire operations using [Controller](https://dataclient.io/docs/api/Controller)
 - `normalizr` - benchmarks just normalize/denormalize
+  - `denormalizeLong nested 25% missing|null|INVALID` (and `withCache`) - the project list with every 4th nested `BuildTypeDescription` missing from the store, `null`, or `INVALID`; those are still tracked as memo dependencies
 - `spread` - degenerate-case writes where spread cost scales with store size (single-entity `setResponse` into 1k/10k/100k entity stores, 10k cached endpoint keys, collection push onto 10k items, invalidateAll/expireAll). One case (`setOneEntity in 10k entity store`) is tracked over time in CI via `.github/workflows/benchmark-spread.yml`; the rest are manual-only.
 - `micro` - isolated microbenchmarks for testing specific operations
 - `old-normalizr` - runs equivalent benchmarks using the normalizr package

examples/benchmark/normalizr.js

@@ -6,6 +6,7 @@ import {
   initialState,
   MemoCache,
   WeakDependencyMap,
+  INVALID,
 } from './dist/index.js';
 import { createAdd } from './filter.js';
 import { printStatus } from './printStatus.js';
@@ -39,6 +40,23 @@ const dataValues = {
 };
 const valuesState = normalize(ProjectSchemaValues, dataValues);
 
+/** `entities` with every 4th nested BuildTypeDescription missing, or set to
+ * `hole`. Those are still dependencies of the projects that nest them. */
+function withNestedHoles(hole) {
+  const buildTypes = { ...entities.BuildTypeDescription };
+  Object.keys(buildTypes).forEach((id, i) => {
+    if (i % 4) return;
+    if (hole === 'missing') delete buildTypes[id];
+    else buildTypes[id] = hole;
+  });
+  return { ...entities, BuildTypeDescription: buildTypes };
+}
+const nestedHoles = [
+  ['missing', withNestedHoles('missing')],
+  ['null', withNestedHoles(null)],
+  ['INVALID', withNestedHoles(INVALID)],
+];
+
 let githubState = normalize(User, userData);
 
 const date = Date.now();
@@ -144,6 +162,11 @@ export default function addNormlizrSuite(suite, filter) {
   add('denormalizeLong donotcache', () => {
     return denormalize(ProjectSchema, result, entities);
   });
+  for (const [hole, holeEntities] of nestedHoles) {
+    add(`denormalizeLong nested 25% ${hole}`, () => {
+      return new MemoCache().denormalize(ProjectSchema, result, holeEntities);
+    });
+  }
   add('denormalizeLong Values donotcache', () => {
     return denormalize(
       ProjectSchemaValues,
@@ -202,6 +225,14 @@ export default function addNormlizrSuite(suite, filter) {
       },
     },
   );
+  for (const [hole, holeEntities] of nestedHoles) {
+    // one MemoCache each, so the variants don't evict each other's result
+    const memoHoles = new MemoCache();
+    memoHoles.denormalize(ProjectSchema, result, holeEntities, []);
+    add(`denormalizeLong nested 25% ${hole} withCache`, () => {
+      return memoHoles.denormalize(ProjectSchema, result, holeEntities, []);
+    });
+  }
   add('denormalizeLong Values withCache', () => {
     return memo.denormalize(
       ProjectSchemaValues,

examples/benchmark/src/index.ts

@@ -3,6 +3,7 @@ export {
   denormalize,
   WeakDependencyMap,
   MemoCache,
+  INVALID,
 } from '@data-client/normalizr';
 export * from '@data-client/core';
 export {

packages/normalizr/AGENTS.md

@@ -11,6 +11,22 @@ Non-obvious constraints when changing `src/memo/`, `src/denormalize/`, or `src/i
 - **Storage shape is API.** `state.entities[key][pk]` is observed by snapshot tests, SSR payloads, and devtools. Shape changes are breaking even when normalize/denormalize round-trip correctly.
 - **Cache invalidation is reference-driven.** A new object at `entities[key][pk]` busts every chain through that step. Storing multiple logical cells under one `pk` invalidates them all on any single-cell write.
 
+## Memo invariants
+
+Every change to `src/memo/`, or to a schema's `denormalize`, must keep these. Tests: `src/__tests__/WeakDependencyMap.test.ts`, `src/__tests__/MemoCache.ts`, `packages/react/src/__tests__/concurrent-memo.web.tsx`.
+
+1. **Sound hits.** A hit returns only a value computed from exactly these entity objects (`===`; non-object values such as `undefined`, `null` and `INVALID` compare by value) and these `argsKey` results.
+2. **Complete dependencies.** Every entity and `argsKey` a computation reads is recorded, including deps replayed from a nested hit, and including entities that are missing, `null` or `INVALID`.
+3. **Deterministic.** The same reads lead to the same further reads and an equal result. All args-dependence goes through a pure, referentially stable `argsKey` fn.
+4. **Complete journey.** The paths returned with a hit cover every entity the value depends on; subscriptions and GC count refs from them. Callers never mutate it.
+5. **Weak retention.** A cached value lives only while what it is keyed on is reachable.
+6. **Stable identity.** The same reads return the same object while it is cached.
+
+Accepted limitations (intentional; don't "fix"):
+
+- The results cache is keyed by the input object, not the schema. Two schemas only see the same result object if two endpoints share a cache key, which is already a bug in the endpoints.
+- Entities past `MAX_ENTITY_DEPTH` (`src/denormalize/unvisit.ts`) are read but not tracked, so changes that deep don't invalidate ancestors.
+
 ## Schema-tree traversal pattern
 
 When walking the schema *definition* (not data), the canonical recursion is used in many places. Keep it consistent:

packages/normalizr/src/__tests__/MemoCache.ts

@@ -6,9 +6,11 @@ import {
   CoolerArticle,
   IndexedUser,
   FirstUnion,
+  Article,
 } from '__tests__/new';
 
 import { fromJSState } from './immutable.test';
+import { INVALID } from '../denormalize/symbol';
 import { IQueryDelegate } from '../interface';
 import MemoCache from '../memo/MemoCache';
 import { MemoPolicy as POJOPolicy } from '../memo/Policy';
@@ -1063,3 +1065,55 @@ describe('MemoCache', () => {
     });
   });
 });
+
+describe('MemoCache nested entity dependencies', () => {
+  const article = { id: '1', title: 't', author: '2' };
+  const bob = { id: '2', username: 'bob' };
+
+  it.each([
+    ['missing', {}],
+    ['INVALID', { '2': INVALID }],
+    ['null', { '2': null }],
+  ])(
+    'a %s child is a dependency: the parent recomputes once it is set',
+    (_, users) => {
+      const memo = new MemoCache();
+      const first = memo.denormalize(Article, '1', {
+        Article: { '1': article },
+        User: users,
+      });
+      expect((first.data as any).author?.username).toBeUndefined();
+      const second = memo.denormalize(Article, '1', {
+        Article: { '1': article },
+        User: { '2': bob },
+      });
+      expect((second.data as any).author?.username).toBe('bob');
+    },
+  );
+
+  it('a blank child id is not a dependency', () => {
+    const memo = new MemoCache();
+    const entities = {
+      Article: { '1': { ...article, author: '' } },
+      User: {},
+    };
+    const first = memo.denormalize(Article, '1', entities);
+    expect((first.data as any).author).toBeUndefined();
+    const second = memo.denormalize(Article, '1', {
+      ...entities,
+      User: { '2': bob },
+    });
+    expect(second.data).toBe(first.data);
+  });
+
+  it('stays cached while the child stays INVALID', () => {
+    const memo = new MemoCache();
+    const entities = { Article: { '1': article }, User: { '2': INVALID } };
+    const first = memo.denormalize(Article, '1', entities);
+    const second = memo.denormalize(Article, '1', {
+      ...entities,
+      User: { ...entities.User },
+    });
+    expect(second.data).toBe(first.data);
+  });
+});

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

@@ -1,8 +1,9 @@
 import { Temporal } from 'temporal-polyfill';
 
+import { INVALID } from '../denormalize/symbol';
 import { EntityPath } from '../interface';
 import { MemoPolicy } from '../memo/Policy';
-import WeakDependencyMap from '../memo/WeakDependencyMap';
+import WeakDependencyMap, { depKey } from '../memo/WeakDependencyMap';
 
 describe('WeakDependencyMap', () => {
   const a = { hi: '5' };
@@ -91,6 +92,32 @@ describe('WeakDependencyMap', () => {
     expect(wem.get({}, getEntity)[0]).toBeUndefined();
   });
 
+  it.each<[string, any]>([
+    ['null', null],
+    ['INVALID', INVALID],
+  ])('tracks a %s dependency, as root or after it', (_, value) => {
+    const wem = new WeakDependencyMap<EntityPath>();
+    const withA = (entity: any) =>
+      MemoPolicy.getEntities({ ...state, A: { '1': entity } });
+    const depValue = { path: depA.path, entity: value };
+    wem.set([depValue], 'root');
+    wem.set([depB, depValue], 'child');
+    expect(wem.get(value, withA(value))[0]).toBe('root');
+    expect(wem.get(b, withA(value))[0]).toBe('child');
+    expect(wem.get(b, withA(a))[0]).toBeUndefined();
+    expect(wem.get(b, withA(undefined))[0]).toBeUndefined();
+  });
+
+  it('keys objects and functions by themselves, so they stay collectable', () => {
+    // an Entity class is a function, and is the first dependency of buildQueryKey
+    class Thing {}
+    const obj = {};
+    expect(depKey(Thing)).toBe(Thing);
+    expect(depKey(obj)).toBe(obj);
+    expect(depKey(null)).toBe(depKey(null));
+    expect(depKey(null)).not.toBe(depKey(INVALID));
+  });
+
   it('considers empty key list invalid', () => {
     const wem = new WeakDependencyMap<EntityPath>();
     expect(() => wem.set([], 'whatever')).toThrowErrorMatchingInlineSnapshot(
@@ -156,7 +183,7 @@ describe('WeakDependencyMap', () => {
       expect(wem.get(a, getEntity, [{ userId: 'alice' }])[0]).toBeUndefined();
 
       // removing entity `b` from the store makes the tail entity lookup
-      // resolve to the UNDEF sentinel in mixed mode, which doesn't match
+      // resolve to the undefined stand-in in mixed mode, which doesn't match
       // the originally-recorded reference → miss
       const getEntityMissingB = MemoPolicy.getEntities({ A: { '1': a } });
       expect(

packages/normalizr/src/denormalize/cache.ts

@@ -17,6 +17,9 @@ export default interface Cache {
     data: any;
     paths: EntityPath[];
   };
+  /** Records an entity that isn't an object (missing, `null` or `INVALID`)
+   * as a dependency of the surrounding entity-cache frame. */
+  dependOn(pk: string, schema: EntityInterface, entity: unknown): void;
   /** Records `fn(args)` as a memoization dimension for the surrounding
    * entity-cache frame and returns the value. */
   argsKey(fn: KeyFn): string | undefined;

packages/normalizr/src/denormalize/localCache.ts

@@ -29,6 +29,8 @@ export default class LocalCache implements Cache {
     return localCacheKey.get(pk);
   }
 
+  dependOn(): void {}
+
   getResults(
     input: any,
     cachable: boolean,

packages/normalizr/src/denormalize/unvisit.ts

@@ -1,6 +1,5 @@
 import type Cache from './cache.js';
 import { INVALID } from './symbol.js';
-import { UNDEF } from './UNDEF.js';
 import type {
   EntityInterface,
   EntityPath,
@@ -11,6 +10,18 @@ import type { DenormGetEntity } from '../memo/types.js';
 import { denormalize as arrayDenormalize } from '../schemas/Array.js';
 import { denormalize as objectDenormalize } from '../schemas/Object.js';
 
+/** Ids that can't be looked up, so they're never cached or tracked. */
+const isBlankPk = (pk: unknown) =>
+  pk === undefined || pk === '' || pk === 'undefined';
+
+/** What a missing, `null` or `INVALID` entity denormalizes to. */
+const nonObjectValue = (
+  schema: EntityInterface,
+  entity: unknown,
+  delegate: IDenormalizeDelegate,
+): any =>
+  typeof entity === 'symbol' ? schema.denormalize(entity, delegate) : entity;
+
 const getUnvisitEntity = (
   getEntity: DenormGetEntity,
   cache: Cache,
@@ -23,28 +34,14 @@ const getUnvisitEntity = (
     const inputIsId = typeof entityOrId !== 'object';
     const entity =
       inputIsId ? getEntity({ key: schema.key, pk: entityOrId }) : entityOrId;
-    if (typeof entity === 'symbol') {
-      return schema.denormalize(entity, delegate);
-    }
-
-    if (
-      entity === undefined &&
-      inputIsId &&
-      // entityOrId cannot be undefined literal as this function wouldn't be called in that case
-      // however the blank strings can still occur
-      entityOrId !== '' &&
-      entityOrId !== 'undefined'
-    ) {
-      // we cannot perform WeakMap lookups with `undefined`, so we use a special object to represent undefined
-      // we're actually using this call to ensure we update the cache if a nested schema changes from `undefined`
-      // this is because cache.getEntity adds this key,pk as a dependency of anything it is nested under
-      return cache.getEntity(entityOrId, schema, UNDEF, localCacheKey => {
-        localCacheKey.set(entityOrId, undefined);
-      });
-    }
-
     if (typeof entity !== 'object' || entity === null) {
-      return entity as any;
+      // A missing, null or INVALID entity is still a dependency of anything
+      // nested above it, so those recompute once it changes. Blank ids can't
+      // be looked up, so they aren't dependencies.
+      // Only the dependency is needed: there is nothing worth caching.
+      if (inputIsId && !isBlankPk(entityOrId))
+        cache.dependOn(entityOrId, schema, entity);
+      return nonObjectValue(schema, entity, delegate);
     }
 
     let pk: string | number | undefined =
@@ -54,7 +51,7 @@ const getUnvisitEntity = (
 
     // if we can't generate a working pk we cannot do cache lookups properly,
     // so simply denormalize without caching
-    if (pk === undefined || pk === '' || pk === 'undefined') {
+    if (isBlankPk(pk)) {
       return noCacheGetEntity(localCacheKey =>
         unvisitEntityObject(schema, entity, '', localCacheKey, delegate),
       );