diff --git a/AGENTS.md b/AGENTS.md index 7b4baee..56f5d5f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -203,6 +203,12 @@ Preserve these unless the task explicitly changes them. the baseline advances. Untouched fields follow the server; the client's own edits survive; same-field concurrent edits stay last-write-wins (local edit preserved and re-sent). Do not gate this rebase on `waitingForUpdate`. +- State is structurally shared and immutable. Mutations copy only the changed + path (`applyDiff`), snapshots reuse every equal document and nested object of + the current view (`replaceEqualDeep`), and the rebase does the same against + the pending `localState`. Never mutate `syncState`, `localState`, or published + data in place; outside production `notify()` deep-freezes them so a mutation + throws. - Collections enforce deletes-win: a doc in the baseline but absent from the new snapshot was deleted remotely → drop it (and any local edits to it) and never recreate it. A doc absent from the baseline but present locally is a @@ -222,6 +228,8 @@ Add or update focused tests near the behavior being changed: `src/__tests__/conflict-resolution.test.ts`, `src/__tests__/reconcile.test.ts`, and `src/__tests__/fieldpath.test.ts`. - Diff behavior: `src/utils/diff.test.ts`. +- Structural sharing and dev-mode freezing (identity across edits, snapshots, + and rebases): `src/__tests__/structural-sharing.test.ts`. - Undo behavior: `src/utils/undo.test.ts`. - Store/global sync behavior: `src/core/store.test.ts`. - React hook surface — selectors, shared subscriptions, `queryConstraints` diff --git a/README.md b/README.md index c9a69c4..460ffb4 100644 --- a/README.md +++ b/README.md @@ -320,6 +320,15 @@ When you call `update()`, the change is applied immediately to local state. The 3. Sends only changed fields to Firestore using dot-notation (flattened keys) 4. Handles any conflicts from concurrent changes +State is structurally shared. An edit copies only the objects on the changed +path, and a snapshot reuses every document and nested object whose value did +not change. Unchanged data keeps its identity, so `React.memo` and `useMemo` +keyed on it hold across edits, write confirmations, and collaborator changes. + +Treat the data as immutable. Outside production builds, Firestate deep-freezes +its state, so a mutation such as `space.name = 'x'` throws a `TypeError`. +Change data through `update`, `set`, `add`, or `remove`. + ### Update vs Set Firestate uses Firestore's `updateDoc` for partial updates and `setDoc` for full replacements: @@ -726,9 +735,8 @@ const { // queryConstraints are keyed by query identity, not array reference: the // subscription rebuilds only when the query actually changes (compared via // Firestore's queryEqual). So a new array that produces the same query — -// e.g. stationIds read from a document Firestate deep-clones on every -// optimistic update — does NOT tear down the listener. You don't need to -// memoize for correctness: +// e.g. stationIds rebuilt from document data on every render — does NOT +// tear down the listener. You don't need to memoize for correctness: const stations = useCollection({ definition: weatherStations, enabled: stationIds.length > 0, @@ -973,7 +981,8 @@ import { // Compute minimal diff between two objects const diff = computeDiff(oldState, newState) -// Apply diff (returns new object, original unchanged) +// Apply diff (returns new object, original unchanged; unchanged +// subtrees are shared with the original, so treat both as immutable) const newState = applyDiff(currentState, diff) // Apply diff in place (mutates target object) - use for performance-critical paths diff --git a/docs/api-recipes.md b/docs/api-recipes.md index e375d54..93e626c 100644 --- a/docs/api-recipes.md +++ b/docs/api-recipes.md @@ -227,10 +227,10 @@ const spaces = useSpaces({ projectId }, { queryConstraints }) ### Dynamic queries built from document data A subtlety with the `useMemo` recipe above: `useMemo` keys on its dependencies -*by reference*. If a dependency is an array or object read out of another -Firestate document, its reference changes on every optimistic update to that -document — Firestate deep-clones local state on edit — even when the contents -are identical. The memo then produces a new constraints array on each edit. +*by reference*. Firestate keeps the identity of data an edit does not change, +but an array derived from document data (for example with `.map` or `.filter`) +is new on every render, even when its contents are identical. A memo keyed on +it then produces a new constraints array each time. `useCollection` handles this for you. It keys the subscription on the *semantic identity* of the query, not the array reference: it builds the query @@ -309,9 +309,10 @@ From a narrowed handle, prefer `update`; use `set` only when you hold the full document. The slice defaults to a deep value comparison, so returning a fresh object/array -of the same shape does not over-render. This matters for collection -sub-selection: an unchanged document may not keep object identity across an -optimistic rebase, but the default comparison still treats it as equal. Pass +of the same shape does not over-render. Unchanged documents also keep their +object identity across edits and snapshots, so a selector that returns one +document (`s.data[id]`) returns the same reference until that document changes. +Pass `isEqual: shallow` for a cheaper one-level compare on flat projections, or a custom comparator: diff --git a/docs/architecture.md b/docs/architecture.md index dc71af5..de091e7 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -47,6 +47,15 @@ merged = localState ?? syncState For documents, `localState === null` represents a pending delete and surfaces as `data: undefined`. +State is structurally shared and immutable. A mutation copies only the objects +on the changed path (`applyDiff`). A snapshot runs through `replaceEqualDeep` +against the current view (`localState ?? syncState`), so every document and +nested object whose value did not change keeps its identity, including the +document a confirmed write echoes back. The rebased `localState` is shared the +same way. Outside production, `notify()` deep-freezes `syncState` and +`localState` (`freezeDeepInDevelopment`), so any in-place mutation — internal or +in app code — throws instead of corrupting the diff baseline. + ## Shared subscriptions `src/core/shared-subscription.ts` is a registry that lets many hooks share one @@ -243,8 +252,8 @@ Important details: reference. `QueryConstraint` objects are opaque, so Firestate never hand-rolls a deep compare; instead `useCollection` builds the query and compares it with Firestore's own `queryEqual`. When upstream state churns - array references without changing the query (e.g. ids read from a - deep-cloned document), the listener is preserved; a genuine query change + array references without changing the query (e.g. ids derived from + document data on each render), the listener is preserved; a genuine query change rebuilds it. Callers need not memoize the array for correctness. - Subscription state and handles are both cached until state changes (`getState()` and `getHandle()` return identity-stable references, rebuilt only on `notify()`), @@ -328,6 +337,8 @@ Collection sync uses a Firestore write batch: - arrays are replaced as whole values - nested plain objects are recursively diffed - `Timestamp` values are compared and cloned specially +- `applyDiff` and `applyOverridesAtPaths` copy only the changed path +- `replaceEqualDeep` reuses every equal subtree of the previous value - Firestore sentinels are preserved while flattening These helpers are shared by document sync, collection sync, and undo. diff --git a/package.json b/package.json index 546f9af..83dd265 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@hvakr/firestate", - "version": "0.1.7", + "version": "0.2.0", "description": "Firestore state management for React with real-time sync, undo/redo, optimistic updates, and Zod schema validation", "author": "HVAKR", "license": "MIT", diff --git a/src/__tests__/structural-sharing.test.ts b/src/__tests__/structural-sharing.test.ts new file mode 100644 index 0000000..92508d8 --- /dev/null +++ b/src/__tests__/structural-sharing.test.ts @@ -0,0 +1,255 @@ +/** + * Structural sharing: an edit or a snapshot keeps the identity of every + * document and nested object it does not change, so reference-based + * memoization (`React.memo`, `useMemo`) in consumers holds. Published state is + * frozen outside production, so code that mutates shared state throws. + */ +import { vi, describe, it, expect, beforeEach } from 'vitest' + +vi.mock('firebase/firestore', async () => { + const actual = + await vi.importActual( + 'firebase/firestore' + ) + const { buildFirestoreMock } = await import('./test-harness') + return buildFirestoreMock(actual as unknown as Record) +}) + +import { createHarness, type Harness } from './test-harness' +import { createCollectionSubscription } from '../core/collection' +import { createDocumentSubscription } from '../core/document' +import { defineCollection, defineDocument } from '../registry/schema' +import { createStore, type FirestateStore } from '../core/store' + +interface Space { + id?: string + name?: string + occupancy?: number + processed?: boolean + edges?: { a: { x: number; y: number } } +} + +const spaces = defineCollection({ path: 'spaces' }) + +interface Project { + name?: string + building?: { floors: number } + weather?: { station: string } +} + +const projectDef = defineDocument({ collection: 'projects', id: 'p1' }) + +const serverSpaces = () => ({ + A: { name: 'Janitor', occupancy: 0, edges: { a: { x: 0, y: 0 } } }, + B: { name: 'Kitchen', occupancy: 2, edges: { a: { x: 1, y: 1 } } }, + C: { name: 'Lounge', occupancy: 4, edges: { a: { x: 2, y: 2 } } }, +}) + +describe('structural sharing', () => { + let store: FirestateStore + let h: Harness + + beforeEach(() => { + vi.clearAllMocks() + vi.spyOn(console, 'error').mockImplementation(() => {}) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + store = createStore({ firestore: {} as never, autosave: 0 }) + h = createHarness() + }) + + const makeColl = () => { + const sub = createCollectionSubscription({ + store, + definition: spaces, + collectionPath: 'spaces', + }) + sub.load() + h.fireCollectionSnapshot(serverSpaces()) + return sub + } + + const makeDoc = () => { + const sub = createDocumentSubscription({ + store, + definition: projectDef, + docId: 'p1', + collectionPath: 'projects', + }) + sub.load() + h.fireDocSnapshot({ + name: 'Student Union', + building: { floors: 2 }, + weather: { station: 'KSEA' }, + }) + return sub + } + + describe('collection', () => { + it('an edit to one document keeps every other document', () => { + const sub = makeColl() + const before = sub.getState().data + + sub.getHandle().update({ A: { occupancy: 4 } }) + + const after = sub.getState().data + expect(after.A?.occupancy).toBe(4) + expect(after.A).not.toBe(before.A) + expect(after.A?.edges).toBe(before.A?.edges) + expect(after.B).toBe(before.B) + expect(after.C).toBe(before.C) + }) + + it('a snapshot that changes one document keeps every other document', () => { + const sub = makeColl() + const before = sub.getState().data + + h.fireCollectionSnapshot({ + ...serverSpaces(), + A: { ...serverSpaces().A, processed: true }, + }) + + const after = sub.getState().data + expect(after.A?.processed).toBe(true) + expect(after.A).not.toBe(before.A) + expect(after.A?.edges).toBe(before.A?.edges) + expect(after.B).toBe(before.B) + expect(after.C).toBe(before.C) + }) + + it('confirming our own write keeps the edited document', async () => { + const sub = makeColl() + sub.getHandle().update({ A: { occupancy: 4 } }) + const edited = sub.getState().data + + void sub.sync() + await h.flushMicrotasks() + h.resolveNextCommit() + await h.flushMicrotasks() + h.fireCollectionSnapshot({ + ...serverSpaces(), + A: { ...serverSpaces().A, occupancy: 4 }, + }) + + const after = sub.getState() + expect(after.isSynced).toBe(true) + expect(after.data.A).toBe(edited.A) + expect(after.data.B).toBe(edited.B) + }) + + it('a server write-back during a pending edit keeps other documents', () => { + const sub = makeColl() + sub.getHandle().update({ A: { occupancy: 4 } }) + const edited = sub.getState().data + + h.fireCollectionSnapshot({ + ...serverSpaces(), + A: { ...serverSpaces().A, processed: true }, + }) + + const after = sub.getState().data + expect(after.A).toMatchObject({ occupancy: 4, processed: true }) + expect(after.A?.edges).toBe(edited.A?.edges) + expect(after.B).toBe(edited.B) + expect(after.C).toBe(edited.C) + }) + + it('add and remove keep every other document', () => { + const sub = makeColl() + const before = sub.getState().data + + sub.getHandle().add('D', { name: 'Office' }) + const added = sub.getState().data + expect(added.D).toMatchObject({ id: 'D', name: 'Office' }) + expect(added.A).toBe(before.A) + + sub.getHandle().remove('B') + const removed = sub.getState().data + expect(removed.B).toBeUndefined() + expect(removed.A).toBe(before.A) + expect(removed.D).toBe(added.D) + }) + + it('undo restores the value and keeps every other document', () => { + const undo: Array<() => void> = [] + const sub = createCollectionSubscription({ + store, + definition: spaces, + collectionPath: 'spaces', + onPushUndo: (undoAction) => undo.push(undoAction), + }) + sub.load() + h.fireCollectionSnapshot(serverSpaces()) + const before = sub.getState().data + + sub.getHandle().update({ A: { occupancy: 4 } }) + undo.pop()?.() + + const after = sub.getState().data + expect(after.A?.occupancy).toBe(0) + expect(after.B).toBe(before.B) + }) + + it('freezes published state', () => { + const sub = makeColl() + const space = sub.getState().data.A! + expect(() => { + space.occupancy = 9 + }).toThrow(TypeError) + expect(() => { + space.edges!.a.x = 9 + }).toThrow(TypeError) + }) + + it('never freezes the caller-owned objects passed to add() or update()', () => { + const sub = makeColl() + const draft = { name: 'New', tags: ['t'], edges: { a: { x: 9, y: 9 } } } + sub.getHandle().add('N', draft as Space) + expect(Object.isFrozen(draft.tags)).toBe(false) + expect(Object.isFrozen(draft.edges.a)).toBe(false) + expect(sub.getState().data.N?.edges).not.toBe(draft.edges) + + const tags = ['u'] + sub.getHandle().update({ A: { tags } as Space }) + expect(Object.isFrozen(tags)).toBe(false) + expect(() => tags.push('v')).not.toThrow() + expect((sub.getState().data.A as Space & { tags: string[] }).tags).toEqual(['u']) + }) + }) + + describe('document', () => { + it('an edit keeps every untouched nested object', () => { + const sub = makeDoc() + const before = sub.getState().data! + + sub.getHandle().update({ building: { floors: 3 } }) + + const after = sub.getState().data! + expect(after.building?.floors).toBe(3) + expect(after.weather).toBe(before.weather) + }) + + it('a snapshot keeps every unchanged nested object', () => { + const sub = makeDoc() + const before = sub.getState().data! + + h.fireDocSnapshot({ + name: 'Renamed', + building: { floors: 2 }, + weather: { station: 'KSEA' }, + }) + + const after = sub.getState().data! + expect(after.name).toBe('Renamed') + expect(after.building).toBe(before.building) + expect(after.weather).toBe(before.weather) + }) + + it('freezes published state', () => { + const sub = makeDoc() + const data = sub.getState().data! + expect(() => { + data.building!.floors = 9 + }).toThrow(TypeError) + }) + }) +}) diff --git a/src/core/collection.ts b/src/core/collection.ts index 828d5ab..a062027 100644 --- a/src/core/collection.ts +++ b/src/core/collection.ts @@ -24,15 +24,16 @@ import type { import type { FirestateStore } from './store' import { applyDiff, - applyDiffMutable, applyOverridesAtPaths, computeDiff, deepClone, diffToFieldPathArgs, dropCommittedSentinels, + freezeDeepInDevelopment, isDeepEqual, observableStateChanged, reconcileDisplayOverrides, + replaceEqualDeep, valuesEqualForNoOp, } from '../utils/diff' @@ -40,6 +41,27 @@ import { // even when multiple instances target the same collection path. let syncKeyCounter = 0 +/** + * Ensure every document carries its own id. Copies only a document whose id + * is missing or wrong (and the record holding it), so state stays shared. + */ +const withDocIds = ( + data: Record +): Record => { + let result = data + for (const [docId, docData] of Object.entries(data)) { + if ( + docData && + typeof docData === 'object' && + (docData as Record).id !== docId + ) { + if (result === data) result = { ...data } + result[docId] = { ...docData, id: docId } + } + } + return result +} + /** * Build the Firestore query a collection subscription runs: `definition`-level * constraints first, then hook-level `extraConstraints`. With no constraints at @@ -236,6 +258,10 @@ export const createCollectionSubscription = ( observableStateChanged(prev, next) const notify = () => { + // State is structurally shared between syncState, localState, and + // every published snapshot, so it must never be mutated in place. + freezeDeepInDevelopment(state.syncState) + freezeDeepInDevelopment(state.localState) // Reconcile display overrides against the current localState // before publishing — see document.ts for the full contract. reconcileDisplayOverrides( @@ -283,16 +309,15 @@ export const createCollectionSubscription = ( // in localState survive into newLocalState. getMergedData() substitutes // display-override Timestamps at sentinel paths, which would erase the // sentinel from state.localState on the next update() call. + // applyDiff copies only the changed documents, so every untouched + // document keeps its identity for downstream memoization. const rawBase = state.localState ?? state.syncState ?? {} - const newLocalState = deepClone(rawBase) - applyDiffMutable(newLocalState, diff as Record) - - // Ensure each document has its id - for (const [docId, docData] of Object.entries(newLocalState)) { - if (docData && typeof docData === 'object') { - ;(docData as Record).id = docId - } - } + const newLocalState = withDocIds( + applyDiff( + rawBase as FirestoreObject, + diff as WithFieldValue> + ) as Record + ) // No-op collapse (§3): a write whose merged result equals the current // view produces NO new state identity, undo entry, notify, or @@ -372,12 +397,13 @@ export const createCollectionSubscription = ( // unknown keys via Zod's default `.strip()` and re-transform on // undo/redo replay. We feed `{ ...data, id }` to parse so the same // validation works whether the user's schema declares `id` or not. - const newDoc = { ...data, id } as unknown as TData + // Copy the caller's data (like set() does): state is frozen outside + // production, and the caller's object must stay theirs to edit. + const newDoc = { ...deepClone(data), id } as unknown as TData if (schema) schema.parse(newDoc) const currentData = getMergedData() - const newLocalState = deepClone(currentData) - newLocalState[id] = newDoc + const newLocalState = { ...currentData, [id]: newDoc } // No-op collapse (§3): adding an id that already holds identical data // is a complete no-op. The returned id stays meaningful (the doc @@ -419,7 +445,7 @@ export const createCollectionSubscription = ( const currentData = getMergedData() if (!(id in currentData)) return - const newLocalState = deepClone(currentData) + const newLocalState = { ...currentData } delete newLocalState[id] // Push undo eagerly. Inverse diff re-adds the removed doc. @@ -544,15 +570,23 @@ export const createCollectionSubscription = ( } const handleSnapshot = (docs: Array<{ id: string; data: TData }>) => { - const newSyncState: Record = {} + const snapshotState: Record = {} for (const { id, data } of docs) { - newSyncState[id] = { ...data, id } as TData + snapshotState[id] = { ...data, id } as TData } // `prevSync` is the BASELINE: the server snapshot our pending local // edits are measured against. Advanced to `newSyncState` here, after // capturing it for the rebase below. const prevSync = state.syncState + const currentLocal = state.localState + // Every snapshot delivers new objects for every document. Reuse each + // object that is equal to the current view, so a snapshot that changes + // one document (or confirms our own write) leaves the rest untouched. + const newSyncState = replaceEqualDeep( + currentLocal ?? prevSync, + snapshotState + ) state.syncState = newSyncState // A successful snapshot supersedes any previous read or write error. state.error = undefined @@ -568,8 +602,6 @@ export const createCollectionSubscription = ( // snapshot from another client left `localState` on a stale base, so // the next sync re-wrote untouched fields (collaborator clobber) and // recreated remotely-deleted docs (delete resurrection). - const currentLocal = state.localState - if (currentLocal !== undefined && prevSync !== undefined) { // Field-level merge: re-derive the user's OWN edits relative to the // baseline and re-apply only those over the new server truth. @@ -585,7 +617,7 @@ export const createCollectionSubscription = ( userEdits as Record, committed as Record | undefined ) - const rebasedLocalState = applyDiff( + const rebased = applyDiff( newSyncState as FirestoreObject, userEdits ) as Record @@ -595,19 +627,21 @@ export const createCollectionSubscription = ( // any pending local edits to it, and never recreate it. This is // unconditional: classified against the baseline, not the rebased // result, so a stale local copy can't resurrect a deleted doc. - for (const docId of Object.keys(prevSync)) { - if (!(docId in newSyncState)) { - delete rebasedLocalState[docId] - } - } + const remotelyDeleted = Object.keys(prevSync).filter( + (docId) => !(docId in newSyncState) && docId in rebased + ) + const withoutDeleted = remotelyDeleted.length + ? { ...rebased } + : rebased + for (const docId of remotelyDeleted) delete withoutDeleted[docId] // Re-add ids (applyDiff may have introduced docs from the snapshot - // and merged edits onto them). - for (const [docId, docData] of Object.entries(rebasedLocalState)) { - if (docData && typeof docData === 'object') { - ;(docData as Record).id = docId - } - } + // and merged edits onto them), then keep the identity of every + // pending document the rebase left unchanged. + const rebasedLocalState = replaceEqualDeep( + currentLocal, + withDocIds(withoutDeleted) + ) // If the rebase leaves nothing that differs from the server, the // edits are fully absorbed → drop localState so isSynced flips back diff --git a/src/core/document.ts b/src/core/document.ts index a96a723..40fbf61 100644 --- a/src/core/document.ts +++ b/src/core/document.ts @@ -22,15 +22,16 @@ import type { import type { FirestateStore } from './store' import { applyDiff, - applyDiffMutable, applyOverridesAtPaths, computeDiff, deepClone, diffToFieldPathArgs, dropCommittedSentinels, + freezeDeepInDevelopment, isDeepEqual, observableStateChanged, reconcileDisplayOverrides, + replaceEqualDeep, valuesEqualForNoOp, } from '../utils/diff' @@ -240,6 +241,10 @@ export const createDocumentSubscription = ( const publicStateChanged = observableStateChanged const notify = () => { + // State is structurally shared between syncState, localState, and + // every published snapshot, so it must never be mutated in place. + freezeDeepInDevelopment(state.syncState) + freezeDeepInDevelopment(state.localState) // Reconcile display overrides against the current localState // before publishing — captures Timestamp.now() for any newly // arrived serverTimestamp() sentinel and drops entries whose @@ -288,9 +293,10 @@ export const createDocumentSubscription = ( // in localState survive into newLocalState. getMergedData() substitutes // display-override Timestamps at sentinel paths, which would erase the // sentinel from state.localState on the next update() call. + // applyDiff copies only the changed path, so untouched nested + // objects keep their identity for downstream memoization. const rawBase = (state.localState ?? state.syncState) as TData - const newLocalState = deepClone(rawBase) - applyDiffMutable(newLocalState, diff as Record) + const newLocalState = applyDiff(rawBase, diff) // No-op collapse (§3): a write whose merged result equals the current // view must produce NO new state identity, undo entry, notify, or @@ -545,11 +551,18 @@ export const createDocumentSubscription = ( } } - const handleSnapshot = (newSyncData: TData) => { + const handleSnapshot = (snapshotData: TData) => { // `prevSync` is the BASELINE: the server snapshot our pending local // edits are measured against. We advance it to `newSyncData` below, // after using it to re-derive the user's own edits. const prevSync = state.syncState + const currentLocal = state.localState + // Every snapshot delivers new objects. Reuse each nested object that + // is equal to the current view, so unchanged fields keep their identity. + const newSyncData = replaceEqualDeep( + currentLocal ?? prevSync, + snapshotData + ) state.syncState = newSyncData // A successful snapshot supersedes any previous read or write error. state.error = undefined @@ -567,7 +580,6 @@ export const createDocumentSubscription = ( // so the rebase must drop it rather than re-derive it (see below). const committed = state.committedWrite state.committedWrite = undefined - const currentLocal = state.localState if (currentLocal === null) { // Pending delete — our delete intent is the latest word. The doc @@ -594,7 +606,9 @@ export const createDocumentSubscription = ( // edit has been fully absorbed → drop localState so isSynced flips // back to true and no redundant write is queued. const absorbed = isDeepEqual(rebasedLocalState, newSyncData) - state.localState = absorbed ? undefined : rebasedLocalState + state.localState = absorbed + ? undefined + : replaceEqualDeep(currentLocal, rebasedLocalState) } if (minLoadTimeElapsed) { diff --git a/src/utils/diff.test.ts b/src/utils/diff.test.ts index c5084af..3871b8f 100644 --- a/src/utils/diff.test.ts +++ b/src/utils/diff.test.ts @@ -23,6 +23,8 @@ import { extractDiffValue, createDiffAtPath, computeUndoDiff, + freezeDeepInDevelopment, + replaceEqualDeep, } from './diff' describe('diff utilities', () => { @@ -112,6 +114,139 @@ describe('diff utilities', () => { const result = applyDiff(original, diff) expect(result).toEqual({ building: { floors: 10, height: 100 } }) }) + + it('copies only the changed path', () => { + const original = { + a: { x: 1, deep: { y: 2 } }, + b: { z: 3 }, + list: [1, 2], + } + const result = applyDiff(original, { a: { x: 5 } }) + expect(result).toEqual({ + a: { x: 5, deep: { y: 2 } }, + b: { z: 3 }, + list: [1, 2], + }) + expect(result).not.toBe(original) + expect(result.a).not.toBe(original.a) + expect(result.a.deep).toBe(original.a.deep) + expect(result.b).toBe(original.b) + expect(result.list).toBe(original.list) + expect(original.a.x).toBe(1) + }) + + it('returns the input when the diff changes nothing', () => { + const original: { a: { x: number }; name: string; missing?: number } = + { a: { x: 1 }, name: 'same' } + expect(applyDiff(original, { a: { x: 1 }, name: 'same' })).toBe( + original + ) + expect(applyDiff(original, { missing: deleteField() })).toBe( + original + ) + }) + + it('deletes fields without touching siblings', () => { + const original = { a: { x: 1, y: 2 }, b: { z: 3 } } + const result = applyDiff(original, { a: { y: deleteField() } }) + expect(result).toEqual({ a: { x: 1 }, b: { z: 3 } }) + expect(result.b).toBe(original.b) + expect(original.a).toEqual({ x: 1, y: 2 }) + }) + + it('never stores the diff object itself', () => { + const nested = { x: 1 } + const result = applyDiff({} as { a?: { x: number } }, { a: nested }) + expect(result.a).toEqual({ x: 1 }) + expect(result.a).not.toBe(nested) + }) + + it('copies a diff array, but keeps the current one when equal', () => { + const original = { tags: ['a'], list: [{ x: 1 }] } + const tags = ['b'] + const result = applyDiff(original, { tags, list: [{ x: 1 }] }) + expect(result.tags).toEqual(['b']) + expect(result.tags).not.toBe(tags) + expect(result.list).toBe(original.list) + }) + + it('works on frozen input', () => { + const original = { a: { x: 1 }, b: { z: 3 } } + freezeDeepInDevelopment(original) + const result = applyDiff(original, { a: { x: 2 } }) + expect(result).toEqual({ a: { x: 2 }, b: { z: 3 } }) + expect(result.b).toBe(original.b) + }) + }) + + describe('replaceEqualDeep', () => { + it('returns prev when the values are deeply equal', () => { + const prev = { a: { x: 1 }, list: [{ y: 2 }] } + expect( + replaceEqualDeep(prev, { a: { x: 1 }, list: [{ y: 2 }] }) + ).toBe(prev) + }) + + it('reuses every equal subtree of a changed value', () => { + const prev = { a: { x: 1 }, b: { z: 3 }, list: [{ y: 2 }, { y: 3 }] } + const next = { + a: { x: 1 }, + b: { z: 4 }, + list: [{ y: 2 }, { y: 5 }], + } + const result = replaceEqualDeep(prev, next) + expect(result).toEqual(next) + expect(result).not.toBe(prev) + expect(result.a).toBe(prev.a) + expect(result.b).not.toBe(prev.b) + expect(result.list).not.toBe(prev.list) + expect(result.list[0]).toBe(prev.list[0]) + }) + + it('treats added, removed, and explicit-undefined keys as changes', () => { + const prev: Record = { a: 1 } + expect(replaceEqualDeep(prev, { a: 1, b: 2 })).not.toBe(prev) + const withB = { a: 1, b: 2 } + expect(replaceEqualDeep(withB, prev)).not.toBe(withB) + expect(replaceEqualDeep(withB, prev)).toEqual(prev) + expect(replaceEqualDeep(prev, { a: 1, b: undefined })).not.toBe( + prev + ) + expect(replaceEqualDeep([1, 2], [1])).toEqual([1]) + expect(replaceEqualDeep([1], [1, 2])).toEqual([1, 2]) + }) + + it('reuses equal Firestore value types', () => { + const prev = { at: Timestamp.fromMillis(1000) } + const result = replaceEqualDeep(prev, { + at: Timestamp.fromMillis(1000), + }) + expect(result).toBe(prev) + const changed = replaceEqualDeep(prev, { + at: Timestamp.fromMillis(2000), + }) + expect(changed.at.toMillis()).toBe(2000) + }) + + it('returns next for values of a different shape', () => { + const next = { a: 1 } + expect(replaceEqualDeep(undefined, next)).toBe(next) + expect(replaceEqualDeep([1], next)).toBe(next) + }) + }) + + describe('freezeDeepInDevelopment', () => { + it('freezes plain objects and arrays but not Firestore value types', () => { + const at = Timestamp.fromMillis(1000) + const value = { a: { list: [{ x: 1 }] }, at } + freezeDeepInDevelopment(value) + expect(Object.isFrozen(value)).toBe(true) + expect(Object.isFrozen(value.a.list[0])).toBe(true) + expect(Object.isFrozen(at)).toBe(false) + expect(() => { + ;(value.a.list[0] as { x: number }).x = 2 + }).toThrow(TypeError) + }) }) describe('applyDiffMutable', () => { @@ -693,6 +828,22 @@ describe('diff utilities', () => { // a render-time concern only. expect(original.updatedAt).not.toBe(ts) }) + + it('copies only the objects on an override path', () => { + const ts = Timestamp.fromMillis(5000) + const merged = { + a: { meta: { updatedAt: serverTimestamp() }, x: { y: 1 } }, + b: { z: 2 }, + } + const result = applyOverridesAtPaths( + merged, + new Map([['a.meta.updatedAt', ts]]) + ) + + expect(result.a.meta.updatedAt).toBe(ts) + expect(result.a.x).toBe(merged.a.x) + expect(result.b).toBe(merged.b) + }) }) }) }) diff --git a/src/utils/diff.ts b/src/utils/diff.ts index d1ddb01..ffb3b14 100644 --- a/src/utils/diff.ts +++ b/src/utils/diff.ts @@ -543,22 +543,130 @@ export const mergeDiffs = ( * Apply a diff to an object, returning a new object. * The original object is not modified. * + * Structurally shared: only the objects on a changed path are copied. Every + * subtree the diff does not change keeps its identity, and a diff that changes + * nothing returns `state` itself. Treat both the input and the result as + * immutable — they share those subtrees. The diff itself is never stored: its + * plain objects and arrays are copied, so the caller keeps ownership of them. + * * @example * ```ts - * const original = { name: 'Project', count: 5 } + * const original = { name: 'Project', count: 5, building: { floors: 2 } } * const diff = { name: 'Updated', count: deleteField() } * const result = applyDiff(original, diff) - * // result = { name: 'Updated' } - * // original is unchanged + * // result = { name: 'Updated', building: { floors: 2 } } + * // result.building === original.building; original is unchanged * ``` */ export const applyDiff = ( state: T, diff: WithFieldValue> ): T => { - const result = deepClone(state) - applyDiffMutable(result, diff as Record) - return result + const source = state as Record + let result: Record | undefined + + for (const key of Object.keys(diff)) { + const value = (diff as Record)[key] + const current = source[key] + + // `deleteField()` drops the key; see applyDiffMutable. + if (isDeleteField(value)) { + if (key in source) { + result ??= { ...source } + delete result[key] + } + continue + } + + // Opaque values and primitives replace by reference; plain objects + // merge recursively onto the current plain value (or a new one). An + // array keeps the current one when equal, otherwise it is copied so + // the caller's array never becomes (frozen) state. + const next = isPlainObject(value) + ? applyDiff( + (isPlainObject(current) ? current : {}) as FirestoreObject, + value as WithFieldValue> + ) + : Array.isArray(value) + ? isDeepEqual(current, value) + ? current + : deepClone(value) + : value + + if (!(key in source) || !Object.is(next, current)) { + result ??= { ...source } + result[key] = next + } + } + + return (result ?? state) as T +} + +/** + * Return `next`, but reuse every subtree of `prev` that is deeply equal to the + * matching subtree of `next`. When the whole value is equal, `prev` itself is + * returned. Lets a fresh snapshot keep the identity of everything that did not + * change, so reference-based memoization downstream holds. + * + * Plain objects and arrays are walked. Firestore opaque values reuse `prev` + * when `.isEqual` says they match. Equality is strict, like + * {@link isDeepEqual}: an explicit-`undefined` key differs from a missing key. + * + * @internal + */ +export const replaceEqualDeep = (prev: unknown, next: T): T => { + if (Object.is(prev, next)) return next + + if (isFirestoreOpaque(prev) && isFirestoreOpaque(next)) { + return (prev.isEqual(next) ? prev : next) as T + } + + if (Array.isArray(prev) && Array.isArray(next)) { + let equal = prev.length === next.length + const result = next.map((item, index) => { + const shared = replaceEqualDeep(prev[index], item) + if (index >= prev.length || shared !== prev[index]) equal = false + return shared + }) + return (equal ? prev : result) as T + } + + if (isPlainObject(prev) && isPlainObject(next)) { + const nextRecord = next as Record + const keys = Object.keys(nextRecord) + let equal = keys.length === Object.keys(prev).length + const result: Record = {} + for (const key of keys) { + const shared = replaceEqualDeep(prev[key], nextRecord[key]) + if (!(key in prev) || shared !== prev[key]) equal = false + result[key] = shared + } + return (equal ? prev : result) as T + } + + return next +} + +/** + * Freeze every plain object and array reachable from `value`, skipping + * subtrees that are already frozen. Non-production builds freeze published + * state, so code that mutates a structurally shared object throws at once + * instead of silently corrupting the baseline that diffs are computed from. + * Firestore opaque values are left alone. + * + * @internal + */ +export const freezeDeepInDevelopment = (value: unknown): void => { + if (process.env.NODE_ENV === 'production') return + freezeDeep(value) +} + +const freezeDeep = (value: unknown): void => { + if (value === null || typeof value !== 'object') return + if (Object.isFrozen(value)) return + if (!Array.isArray(value) && !isPlainObject(value)) return + Object.freeze(value) + for (const child of Object.values(value)) freezeDeep(child) } /** @@ -820,25 +928,28 @@ export const reconcileDisplayOverrides = ( } } +/** Return a copy of `obj` with `value` at `parts`, copying only that path. */ const setAtPath = ( obj: Record, - path: string, + parts: string[], value: unknown -): void => { - const parts = path.split('.') - let cur = obj - for (let i = 0; i < parts.length - 1; i++) { - const part = parts[i]! - if (!isPlainObject(cur[part])) cur[part] = {} - cur = cur[part] as Record +): Record => { + const [part, ...rest] = parts + if (part === undefined) return obj + const child = obj[part] + return { + ...obj, + [part]: rest.length + ? setAtPath(isPlainObject(child) ? child : {}, rest, value) + : value, } - cur[parts[parts.length - 1]!] = value } /** * Apply a path → value override map to a merged view, returning a new * object. Used by document.ts / collection.ts to substitute display - * values for sentinels still present in `localState`. + * values for sentinels still present in `localState`. Only the objects on an + * override path are copied; everything else keeps its identity. * * @internal */ @@ -846,10 +957,9 @@ export const applyOverridesAtPaths = ( merged: T, overrides: ReadonlyMap ): T => { - if (overrides.size === 0) return merged - const result = deepClone(merged) as Record + let result = merged as Record for (const [path, value] of overrides) { - setAtPath(result, path, value) + result = setAtPath(result, path.split('.'), value) } return result as T }