From bb3343a7ee450097cf5e697e8dad0a1cb70c1de2 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 11 Aug 2026 03:50:58 -0700 Subject: [PATCH] refactor(store): unify the catalog structural-equality walks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three near-identical structural deep-equality walks had landed independently in the same window: areValuesEqual (#13744, repo-identity-reconcile.ts), areCatalogEntriesEqual (#13770, repos.ts — already folded into the first on this branch's base) and catalogValuesEqual (#13662, worktree-catalog-reconciliation.ts). All three walk plain records and arrays and fall back to reference equality for anything exotic. They are not interchangeable. Two axes genuinely differ, and each caller depends on its own side: - Own-key set. #13744/#13770 require equal own-key counts plus hasOwnProperty, so an absent key differs from a key present and holding `undefined`. #13662 compares the union of both sides' keys, so those are equal. The strict side is load-bearing: the repo/project merges branch on `'localWindowsRuntimePreference' in project` (repos-project-runtime.test.ts "clears stale local runtime preferences"), and projects are now reconciled with this comparator. The loose side is test-pinned by worktree-catalog-reconciliation.test.ts "reuses rows with equivalent nested catalog data", where a locally built row carries `optional: undefined` that the host omits. - Leaf comparison. #13744/#13770 use `===` (NaN never equal, 0 equals -0); #13662 uses `Object.is` (the reverse). So instead of picking a winner, src/shared/structural-value-equality.ts holds one walk parameterised by those two axes and exports the two policies as `structuralValuesEqual` and `structuralValuesEqualIgnoringUndefined`. Every caller keeps its exact current semantics; the ~40 duplicated lines and the silent divergence go away. src/shared/persisted-ui-equality.ts (a fourth copy with a Set branch and no plain-object guard) is deliberately left alone: it gates a disk write in main with no direct test coverage. Also folded, all provably behaviour-identical: - The `${hostId}\0${repoId}` composite key had three copies (getRepoHostIdentityForParts, repoOwnerKey, getEntryKey) that must agree or repos silently stop reconciling. Moved to src/shared/repo-host-identity.ts because one of them lives in src/shared; the renderer module re-exports it. - mergeFetchedReposForHost's hand-inlined upsert loop now calls mergeByIdentity. mergeByIdentity additionally skips replacing a structurally equal row, which cannot change the result here: reconcileFetchedRepos runs immediately after over the same identities in the same order and restores exactly those rows. - Renamed repos.ts's `catalogRowsUnchanged` to `arrayElementsUnchanged`. It is a pure element-identity compare, two files away from `catalogRowsEqual`, which is a full structural compare. src/shared/structural-value-equality.test.ts pins both policies over arrays, nested records, null-prototype records, absent-vs-undefined keys, symbol keys, and non-plain objects (Date/Map/Set/class) falling back to reference equality. --- .../src/store/slices/repo-host-identity.ts | 9 +- .../store/slices/repo-identity-reconcile.ts | 44 +-------- src/renderer/src/store/slices/repos.ts | 41 +++----- .../store/slices/superseded-ssh-repo-rows.ts | 19 ++-- .../slices/worktree-catalog-reconciliation.ts | 41 ++------ src/shared/manual-repo-order.ts | 3 +- src/shared/repo-host-identity.ts | 5 + src/shared/structural-value-equality.test.ts | 96 +++++++++++++++++++ src/shared/structural-value-equality.ts | 82 ++++++++++++++++ 9 files changed, 226 insertions(+), 114 deletions(-) create mode 100644 src/shared/repo-host-identity.ts create mode 100644 src/shared/structural-value-equality.test.ts create mode 100644 src/shared/structural-value-equality.ts diff --git a/src/renderer/src/store/slices/repo-host-identity.ts b/src/renderer/src/store/slices/repo-host-identity.ts index f21da2b153a..772d5dc2925 100644 --- a/src/renderer/src/store/slices/repo-host-identity.ts +++ b/src/renderer/src/store/slices/repo-host-identity.ts @@ -4,6 +4,9 @@ import { getSettingsFocusedExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' +import { getRepoHostIdentityForParts } from '../../../../shared/repo-host-identity' + +export { getRepoHostIdentityForParts } type RepoIdentityParts = Pick @@ -11,12 +14,6 @@ export function getRepoHostIdentity(repo: RepoIdentityParts): string { return getRepoHostIdentityForParts(repo.id, getRepoExecutionHostId(repo)) } -export function getRepoHostIdentityForParts(repoId: string, hostId: string): string { - // Why: host ids and repo ids can contain punctuation; NUL keeps the composite - // key collision-free without escaping user/provider-owned strings. - return `${hostId}\0${repoId}` -} - export function repoMatchesHostIdentity( repo: RepoIdentityParts, repoId: string, diff --git a/src/renderer/src/store/slices/repo-identity-reconcile.ts b/src/renderer/src/store/slices/repo-identity-reconcile.ts index db59e619d97..7535422018a 100644 --- a/src/renderer/src/store/slices/repo-identity-reconcile.ts +++ b/src/renderer/src/store/slices/repo-identity-reconcile.ts @@ -1,4 +1,5 @@ import type { Repo } from '../../../../shared/types' +import { structuralValuesEqual } from '../../../../shared/structural-value-equality' import { getRepoHostIdentity } from './repo-host-identity' // Why: after a drag-reorder we optimistically set `repos`, persist, and main @@ -8,44 +9,9 @@ import { getRepoHostIdentity } from './repo-host-identity' // virtualizer to rebuild + re-measure a tick after the drop — the visible jump. // Reusing equal objects (and the whole array when nothing moved) makes the echo // a no-op render. -// Why: `Repo` carries nested records (hookSettings, upstream, gitRemoteIdentity, repoIcon, path -// arrays). IPC structured-clone rebuilds those every fetch, and main's hydrateRepo always -// reconstructs hookSettings — so a reference compare reports every repo as changed and no repo -// ever reconciles. Compare nested plain values structurally; they are small sanitized records. -export function areValuesEqual(a: unknown, b: unknown): boolean { - if (a === b) { - return true - } - if (typeof a !== 'object' || typeof b !== 'object' || a === null || b === null) { - return false - } - if (Array.isArray(a) || Array.isArray(b)) { - return ( - Array.isArray(a) && - Array.isArray(b) && - a.length === b.length && - a.every((item, index) => areValuesEqual(item, b[index])) - ) - } - // Why: only plain records are safe to walk — anything exotic falls back to reference equality. - const aPrototype = Object.getPrototypeOf(a) - const bPrototype = Object.getPrototypeOf(b) - if ( - (aPrototype !== Object.prototype && aPrototype !== null) || - (bPrototype !== Object.prototype && bPrototype !== null) - ) { - return false - } - const aRecord = a as Record - const bRecord = b as Record - const keys = Object.keys(aRecord) - if (keys.length !== Object.keys(bRecord).length) { - return false - } - return keys.every( - (key) => Object.hasOwn(bRecord, key) && areValuesEqual(aRecord[key], bRecord[key]) - ) -} +// Why the structural compare: `Repo` carries nested records (hookSettings, upstream, +// gitRemoteIdentity, repoIcon, path arrays) that structured-clone and main's hydrateRepo rebuild +// every fetch, so a reference compare would report every repo as changed. /** * Reuses equal rows from `previous` — and the whole array when nothing moved — so a refetch that @@ -61,7 +27,7 @@ export function reconcileCatalogRows( let identical = next.length === previous.length const reconciled = next.map((row, index) => { const existing = previousByIdentity.get(getIdentity(row)) - if (existing !== undefined && areValuesEqual(existing, row)) { + if (existing !== undefined && structuralValuesEqual(existing, row)) { if (existing !== previous[index]) { identical = false } diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 059baa6dde5..0b8f0dfcb68 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -48,12 +48,9 @@ import { applyManualRepoOrder, getManualRepoOrder } from '../../../../shared/man import { getProjectGroupSubtreeIds } from '../../../../shared/project-groups' import { isPathInsideOrEqual } from '../../../../shared/cross-platform-path' import { getRepoIdFromWorktreeId } from '../../../../shared/worktree/id' +import { structuralValuesEqual } from '../../../../shared/structural-value-equality' import { selectProjectGroupRemovalTargets } from './project-group-removal-targets' -import { - areValuesEqual, - reconcileCatalogRows, - reconcileFetchedRepos -} from './repo-identity-reconcile' +import { reconcileCatalogRows, reconcileFetchedRepos } from './repo-identity-reconcile' import { retainValidFilterRepoIds } from './repo-filter-selection' import { mergeSshRepoReadoptions, @@ -1022,7 +1019,7 @@ function mergeByIdentity( changed = true continue } - if (areValuesEqual(merged[index], entry)) { + if (structuralValuesEqual(merged[index], entry)) { continue } merged[index] = entry @@ -1043,7 +1040,8 @@ function unchangedMergeSource( // Why: the sidebar effect watching these catalog arrays is the only thing that refills the // folder path-status cache, so a no-op refetch must not wipe it — nothing would repopulate it. -function catalogRowsUnchanged(next: readonly T[], previous: readonly T[]): boolean { +// Element identity only; `structuralValuesEqual` is the structural counterpart. +function arrayElementsUnchanged(next: readonly T[], previous: readonly T[]): boolean { return ( next === previous || (next.length === previous.length && next.every((row, index) => row === previous[index])) @@ -1061,19 +1059,10 @@ function mergeFetchedReposForHost( const existingHostId = getRepoExecutionHostId(repo) return existingHostId !== hostId || fetchedIdentities.has(getRepoHostIdentity(repo)) }) - const merged = [...preserved] - const indexByIdentity = new Map(merged.map((repo, index) => [getRepoHostIdentity(repo), index])) - for (const repo of fetchedWithProjectGroups) { - const identity = getRepoHostIdentity(repo) - const existingIndex = indexByIdentity.get(identity) - if (existingIndex === undefined) { - indexByIdentity.set(identity, merged.length) - merged.push(repo) - continue - } - merged[existingIndex] = repo - } - return reconcileFetchedRepos(previous, merged) + return reconcileFetchedRepos( + previous, + mergeByIdentity(preserved, fetchedWithProjectGroups, getRepoHostIdentity) + ) } function applyInheritedProjectGroups(previous: readonly Repo[], fetched: readonly Repo[]): Repo[] { @@ -2135,7 +2124,7 @@ export const createRepoSlice: StateCreator = (set, pendingSshRepoReadoptions: reconciliation.pendingReadoptions, ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), ...mergedProjectCompatibility, - ...(catalogRowsUnchanged(prunedRepos, s.repos) + ...(arrayElementsUnchanged(prunedRepos, s.repos) ? {} : { folderWorkspacePathStatuses: {} }), activeRepoId: s.activeRepoId && validRepoIds.has(s.activeRepoId) ? s.activeRepoId : null, @@ -2288,7 +2277,7 @@ export const createRepoSlice: StateCreator = (set, pendingSshRepoReadoptions: reconciliation.pendingReadoptions, ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), ...mergedProjectCompatibility, - ...(catalogRowsUnchanged(finalizedRepos, s.repos) + ...(arrayElementsUnchanged(finalizedRepos, s.repos) ? {} : { folderWorkspacePathStatuses: {} }), activeRepoId: s.activeRepoId, @@ -2377,7 +2366,7 @@ export const createRepoSlice: StateCreator = (set, const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, current.projectGroups) return { projectGroups, - ...(catalogRowsUnchanged(projectGroups, current.projectGroups) + ...(arrayElementsUnchanged(projectGroups, current.projectGroups) ? {} : { folderWorkspacePathStatuses: {} }) } @@ -2400,7 +2389,7 @@ export const createRepoSlice: StateCreator = (set, const { projectGroups } = mergeFetchedProjectGroupCatalog(catalog, s.projectGroups) return { projectGroups, - ...(catalogRowsUnchanged(projectGroups, s.projectGroups) + ...(arrayElementsUnchanged(projectGroups, s.projectGroups) ? {} : { folderWorkspacePathStatuses: {} }) } @@ -2464,7 +2453,7 @@ export const createRepoSlice: StateCreator = (set, ) return { folderWorkspaces, - ...(catalogRowsUnchanged(folderWorkspaces, current.folderWorkspaces) + ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces) ? {} : { folderWorkspacePathStatuses: {} }) } @@ -2502,7 +2491,7 @@ export const createRepoSlice: StateCreator = (set, ) return { folderWorkspaces, - ...(catalogRowsUnchanged(folderWorkspaces, current.folderWorkspaces) + ...(arrayElementsUnchanged(folderWorkspaces, current.folderWorkspaces) ? {} : { folderWorkspacePathStatuses: {} }) } diff --git a/src/renderer/src/store/slices/superseded-ssh-repo-rows.ts b/src/renderer/src/store/slices/superseded-ssh-repo-rows.ts index c18e0f6e5af..a9880577f60 100644 --- a/src/renderer/src/store/slices/superseded-ssh-repo-rows.ts +++ b/src/renderer/src/store/slices/superseded-ssh-repo-rows.ts @@ -1,6 +1,7 @@ import type { SshRepoReadoption } from '../../../../shared/ssh-types' import type { Repo } from '../../../../shared/types' import { getRepoExecutionHostId, toSshExecutionHostId } from '../../../../shared/execution-host' +import { getRepoHostIdentityForParts } from '../../../../shared/repo-host-identity' export type SshRepoReconciliation = { repos: readonly Repo[] @@ -14,10 +15,6 @@ function repoBelongsToTarget(repo: Repo, targetId: string): boolean { ) } -function repoOwnerKey(hostId: string, repoId: string): string { - return `${hostId}\0${repoId}` -} - /** * Drops old-host rows only when main reports the exact repo re-adoption and the * renderer has received the corresponding new-host row. Evidence stays pending @@ -32,7 +29,7 @@ export function reconcileReadoptedSshRepoRows( const directSshOwners = new Set( repos.flatMap((repo) => repo.connectionId && repoBelongsToTarget(repo, repo.connectionId) - ? [repoOwnerKey(getRepoExecutionHostId(repo), repo.id)] + ? [getRepoHostIdentityForParts(repo.id, getRepoExecutionHostId(repo))] : [] ) ) @@ -40,13 +37,18 @@ export function reconcileReadoptedSshRepoRows( for (const readoption of readoptions) { const pendingRepoIds: string[] = [] for (const repoId of readoption.repoIds) { - const newOwner = repoOwnerKey(toSshExecutionHostId(readoption.newTargetId), repoId) + const newOwner = getRepoHostIdentityForParts( + repoId, + toSshExecutionHostId(readoption.newTargetId) + ) const hasNewRow = directSshOwners.has(newOwner) if (!hasNewRow) { pendingRepoIds.push(repoId) continue } - prunedOwners.add(repoOwnerKey(toSshExecutionHostId(readoption.oldTargetId), repoId)) + prunedOwners.add( + getRepoHostIdentityForParts(repoId, toSshExecutionHostId(readoption.oldTargetId)) + ) } if (pendingRepoIds.length > 0) { pendingReadoptions.push({ ...readoption, repoIds: pendingRepoIds }) @@ -60,7 +62,8 @@ export function reconcileReadoptedSshRepoRows( } return { repos: repos.filter( - (repo) => !prunedOwners.has(repoOwnerKey(getRepoExecutionHostId(repo), repo.id)) + (repo) => + !prunedOwners.has(getRepoHostIdentityForParts(repo.id, getRepoExecutionHostId(repo))) ), pendingReadoptions } diff --git a/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts b/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts index 1ef339a1808..500f002dc59 100644 --- a/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts +++ b/src/renderer/src/store/slices/worktree-catalog-reconciliation.ts @@ -1,41 +1,14 @@ +import { structuralValuesEqualIgnoringUndefined } from '../../../../shared/structural-value-equality' + type CatalogRow = { id: string } -function isPlainRecord(value: unknown): value is Record { - if (typeof value !== 'object' || value === null) { - return false - } - const prototype = Object.getPrototypeOf(value) - return prototype === Object.prototype || prototype === null -} - -function catalogValuesEqual(left: unknown, right: unknown): boolean { - if (Object.is(left, right)) { - return true - } - if (Array.isArray(left) || Array.isArray(right)) { - if (!Array.isArray(left) || !Array.isArray(right) || left.length !== right.length) { - return false - } - return left.every((value, index) => catalogValuesEqual(value, right[index])) - } - if (!isPlainRecord(left) || !isPlainRecord(right)) { - return false - } - const keys = new Set([...Object.keys(left), ...Object.keys(right)]) - for (const key of keys) { - if (!catalogValuesEqual(left[key], right[key])) { - return false - } - } - return true -} - // NOTHING HITS THIS TODAY: both callers key on ids that are unique by // construction, so buckets stay at 1-3. It only bounds the damage if that ever // changes — a same-id bucket costs one deep compare per candidate, so an -// unbounded one is O(k^2). A cap beats a second index that would have to stay in -// step with catalogValuesEqual, and reuse is only an optimization: dropping a -// match past the window costs object identity, never correctness. +// unbounded one is O(k^2). A cap beats an index keyed on a second walk that +// would have to stay in step with structuralValuesEqualIgnoringUndefined, and +// reuse is only an optimization: dropping a match past the window costs object +// identity, never correctness. const MAX_DUPLICATE_ID_SCAN = 8 export function reuseEqualCatalogRows( @@ -62,7 +35,7 @@ export function reuseEqualCatalogRows( const scanLimit = Math.min(candidates.length, MAX_DUPLICATE_ID_SCAN) for (let index = 0; index < scanLimit; index++) { const candidate = candidates[index] - if (candidate !== undefined && catalogValuesEqual(candidate, row)) { + if (candidate !== undefined && structuralValuesEqualIgnoringUndefined(candidate, row)) { candidates.splice(index, 1) return candidate } diff --git a/src/shared/manual-repo-order.ts b/src/shared/manual-repo-order.ts index 9b5bfee39a1..9813412c676 100644 --- a/src/shared/manual-repo-order.ts +++ b/src/shared/manual-repo-order.ts @@ -1,8 +1,9 @@ import { getRepoExecutionHostId, normalizeExecutionHostId } from './execution-host' +import { getRepoHostIdentityForParts } from './repo-host-identity' import type { ManualRepoOrderEntry, Repo } from './types' function getEntryKey(entry: ManualRepoOrderEntry): string { - return `${entry.hostId}\0${entry.repoId}` + return getRepoHostIdentityForParts(entry.repoId, entry.hostId) } export function normalizeManualRepoOrder(value: unknown): ManualRepoOrderEntry[] { diff --git a/src/shared/repo-host-identity.ts b/src/shared/repo-host-identity.ts new file mode 100644 index 00000000000..8309899d4e6 --- /dev/null +++ b/src/shared/repo-host-identity.ts @@ -0,0 +1,5 @@ +export function getRepoHostIdentityForParts(repoId: string, hostId: string): string { + // Why: host ids and repo ids can contain punctuation; NUL keeps the composite + // key collision-free without escaping user/provider-owned strings. + return `${hostId}\0${repoId}` +} diff --git a/src/shared/structural-value-equality.test.ts b/src/shared/structural-value-equality.test.ts new file mode 100644 index 00000000000..57a7a471dd5 --- /dev/null +++ b/src/shared/structural-value-equality.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it } from 'vitest' +import { + structuralValuesEqual, + structuralValuesEqualIgnoringUndefined +} from './structural-value-equality' + +const comparators = [ + ['structuralValuesEqual', structuralValuesEqual], + ['structuralValuesEqualIgnoringUndefined', structuralValuesEqualIgnoringUndefined] +] as const + +describe.each(comparators)('%s', (_name, valuesEqual) => { + it('compares primitives and identical references', () => { + expect(valuesEqual('a', 'a')).toBe(true) + expect(valuesEqual('a', 'b')).toBe(false) + expect(valuesEqual(null, null)).toBe(true) + expect(valuesEqual(null, {})).toBe(false) + expect(valuesEqual(undefined, null)).toBe(false) + const fn = (): void => {} + expect(valuesEqual(fn, fn)).toBe(true) + expect(valuesEqual(fn, (): void => {})).toBe(false) + }) + + it('walks arrays element-wise and never mixes them with records', () => { + expect(valuesEqual([1, [2, { a: 'b' }]], [1, [2, { a: 'b' }]])).toBe(true) + expect(valuesEqual([1, 2], [1, 2, 3])).toBe(false) + expect(valuesEqual([1, 2], [2, 1])).toBe(false) + expect(valuesEqual([], {})).toBe(false) + expect(valuesEqual({ 0: 1, length: 1 }, [1])).toBe(false) + }) + + it('walks nested plain records rebuilt by structured clone', () => { + const record = { id: 'a', nested: { labels: ['one', 'two'], flags: { on: true } } } + expect(valuesEqual(record, structuredClone(record))).toBe(true) + expect( + valuesEqual(record, { ...record, nested: { labels: ['one'], flags: { on: true } } }) + ).toBe(false) + }) + + it('walks null-prototype records like literals', () => { + const nullProto: Record = Object.create(null) + nullProto.a = 1 + expect(valuesEqual(nullProto, { a: 1 })).toBe(true) + expect(valuesEqual(nullProto, { a: 2 })).toBe(false) + }) + + it('falls back to reference equality for non-plain objects', () => { + const date = new Date(0) + expect(valuesEqual(date, date)).toBe(true) + expect(valuesEqual(date, new Date(0))).toBe(false) + expect(valuesEqual(new Map([['a', 1]]), new Map([['a', 1]]))).toBe(false) + expect(valuesEqual(new Set([1]), new Set([1]))).toBe(false) + class Row { + x = 1 + } + expect(valuesEqual(new Row(), new Row())).toBe(false) + expect(valuesEqual(new Row(), { x: 1 })).toBe(false) + }) + + it('ignores symbol keys', () => { + const key = Symbol('marker') + expect(valuesEqual({ [key]: 1, a: 1 }, { [key]: 2, a: 1 })).toBe(true) + }) +}) + +describe('structuralValuesEqual', () => { + it('treats an absent key as different from a key holding undefined', () => { + // Why: repo/project merges branch on `'key' in project`, so key presence is load-bearing. + expect(structuralValuesEqual({ a: 1 }, { a: 1, b: undefined })).toBe(false) + expect(structuralValuesEqual({ a: 1, b: undefined }, { a: 1 })).toBe(false) + expect(structuralValuesEqual({ a: 1, b: undefined }, { a: 1, c: undefined })).toBe(false) + expect(structuralValuesEqual({ a: 1, b: undefined }, { a: 1, b: undefined })).toBe(true) + }) + + it('compares leaves with === so NaN is never equal and -0 matches 0', () => { + expect(structuralValuesEqual({ a: Number.NaN }, { a: Number.NaN })).toBe(false) + expect(structuralValuesEqual({ a: 0 }, { a: -0 })).toBe(true) + }) +}) + +describe('structuralValuesEqualIgnoringUndefined', () => { + it('treats an absent key as equal to a key holding undefined', () => { + // Why: locally built worktree rows carry explicit undefined fields the host omits. + expect(structuralValuesEqualIgnoringUndefined({ a: 1 }, { a: 1, b: undefined })).toBe(true) + expect(structuralValuesEqualIgnoringUndefined({ a: 1, b: undefined }, { a: 1 })).toBe(true) + expect( + structuralValuesEqualIgnoringUndefined({ a: 1, b: undefined }, { a: 1, c: undefined }) + ).toBe(true) + expect(structuralValuesEqualIgnoringUndefined({ a: 1 }, { a: 1, b: 2 })).toBe(false) + }) + + it('compares leaves with Object.is so NaN is equal and -0 differs from 0', () => { + expect(structuralValuesEqualIgnoringUndefined({ a: Number.NaN }, { a: Number.NaN })).toBe(true) + expect(structuralValuesEqualIgnoringUndefined({ a: 0 }, { a: -0 })).toBe(false) + }) +}) diff --git a/src/shared/structural-value-equality.ts b/src/shared/structural-value-equality.ts new file mode 100644 index 00000000000..5714bb7f1ed --- /dev/null +++ b/src/shared/structural-value-equality.ts @@ -0,0 +1,82 @@ +// Why: catalog reconcilers compare rows that IPC structured-clone (and main's hydration) rebuild on +// every fetch, so a reference compare reports every row as changed and nothing ever reconciles. +// Only plain records and arrays are walked; anything exotic (Date, Map, class instance) falls back +// to reference equality rather than being mistaken for an empty record. + +type ValueEqualityPolicy = { + // Why: `Object.is` makes NaN equal NaN but 0 unequal -0; `===` does the opposite. + readonly sameValueLeaves: boolean + readonly absentKeyEqualsUndefined: boolean +} + +const STRICT_OWN_KEYS: ValueEqualityPolicy = { + sameValueLeaves: false, + absentKeyEqualsUndefined: false +} + +const UNION_OF_KEYS: ValueEqualityPolicy = { + sameValueLeaves: true, + absentKeyEqualsUndefined: true +} + +function isPlainRecord(value: unknown): value is Record { + if (typeof value !== 'object' || value === null) { + return false + } + const prototype = Object.getPrototypeOf(value) + return prototype === Object.prototype || prototype === null +} + +function valuesEqual(a: unknown, b: unknown, policy: ValueEqualityPolicy): boolean { + if (policy.sameValueLeaves ? Object.is(a, b) : a === b) { + return true + } + if (Array.isArray(a) || Array.isArray(b)) { + return ( + Array.isArray(a) && + Array.isArray(b) && + a.length === b.length && + a.every((item, index) => valuesEqual(item, b[index], policy)) + ) + } + if (!isPlainRecord(a) || !isPlainRecord(b)) { + return false + } + if (policy.absentKeyEqualsUndefined) { + for (const key of new Set([...Object.keys(a), ...Object.keys(b)])) { + if (!valuesEqual(a[key], b[key], policy)) { + return false + } + } + return true + } + const keys = Object.keys(a) + if (keys.length !== Object.keys(b).length) { + return false + } + return keys.every( + (key) => Object.hasOwn(b, key) && valuesEqual(a[key], b[key], policy) + ) +} + +/** + * Structural compare where an absent own key differs from a key that is present and holds + * `undefined`, and leaves compare with `===`. + * + * Why the strict key set: the repo/project merges branch on `'localWindowsRuntimePreference' in + * project`, so a key appearing or disappearing is a real change even when its value is `undefined`. + */ +export function structuralValuesEqual(a: unknown, b: unknown): boolean { + return valuesEqual(a, b, STRICT_OWN_KEYS) +} + +/** + * Structural compare that treats an absent own key as equal to a key holding `undefined`, and + * compares leaves with `Object.is`. + * + * Why the loose key set: locally constructed worktree catalog rows carry explicit `undefined` + * fields that the host simply omits, and no consumer of those rows uses the `in` operator. + */ +export function structuralValuesEqualIgnoringUndefined(a: unknown, b: unknown): boolean { + return valuesEqual(a, b, UNION_OF_KEYS) +}