Fix deeperEqual throwing when a selection becomes null

[eastagiletracker]

This PR proposes a fix for deeperEqual throwing a TypeError when a selected object or map becomes null. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/297. You can sign in with your GitHub ID to claim ownership of the project.

What is wrong

src/lib/deeper-equal.ts is the isEqual predicate behind two consumers: useStore (src/store/useStore.ts passes it to useSyncExternalStoreWithSelector, and about 45 call sites across src/components and src/hooks go through it), and the exported useManuscriptsState hook (src/hooks/external/use-manuscripts-state.ts, re-exported from src/index.tsx), which a parent app drives with a selector of its own.

Neither of the two opening branches guards the second argument. if (!next) return false only covers the first one, and the typeof prev == 'undefined' check lets null through, so as soon as a selection that is a plain object or a Map turns into null, the Object and Map branches dereference it — prev[i] and prev.size — and throw while React is rendering. A selector as ordinary as (s) => s.files?.find((f) => f.id === id) ?? null is enough to hit it.

The same missing guard also makes the predicate answer "not equal" for a value that did not change: deeperEqual(undefined, undefined), (null, null), (false, false), (0, 0) and ('', '') all return false today.

Reproducing it on master

On master at 29d3cd5, adding the three test files from this PR and running TZ=UTC npx vitest run gives:

TypeError: Cannot read properties of null (reading 'id')
 ❯ deeperEqual src/lib/deeper-equal.ts:24:13
 ❯ src/hooks/external/use-manuscripts-state.ts:76:28
 ❯ useManuscriptsState src/hooks/external/use-manuscripts-state.ts:65:29

TypeError: Cannot read properties of null (reading 'size')
 ❯ deeperEqual src/lib/deeper-equal.ts:34:29
 ❯ src/hooks/external/use-manuscripts-state.ts:76:28

TypeError: Cannot read properties of null (reading 'primaryTab')
 ❯ deeperEqual src/lib/deeper-equal.ts:24:13
 ❯ src/store/useStore.ts:29:5

Nine tests fail on that tree: the five unchanged-falsy cases, the predicate's own nullish case, and the three crashes above — one through useStore, two through the public useManuscriptsState hook.

The change

Six lines in deeperEqual: compare identity first and return true (which covers an unchanged undefined, null, false, 0 or ''), then return false when only one side is nullish, before the structural comparisons that would dereference it. The one-level-deep semantics of the Object, Map, Array and default branches are untouched.

One behavioural note, since this predicate decides re-renders: a component whose selection stays falsy previously reported "changed" on every store update. React's own Object.is bail-out already absorbed that for identical primitives, so no component re-renders that used to — src/store/__tests__/useStore.test.tsx pins that down, including a selection that legitimately moves between undefined, true and false.

How it was verified

TZ=UTC pnpm test, pnpm typecheck and pnpm lint were all run on master before the change (13 tests, all green) and again after it: 34 tests green, tsc --noEmit clean, eslint --max-warnings 0 clean, no new failures either way. The three new test files exercise the predicate directly and both of its consumers — mounting the real useStore and the real useManuscriptsState through react-dom/client — and every one of the nine assertions above fails without the one-file change.

How this was managed

This work was tracked as a story on a board imported from this repository's own issues and pull requests (1222 stories): Fix deeperEqual throwing when a store selection becomes null, on the board at eastagiletracker.com/projects/297.

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.

---

Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com