Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-view): depend ObjectView's non-grid fetch on the view's filter and sort, not the view object - #6582
Conversation
…sort, not the view object
`ObjectView`'s non-grid fetch effect listed `activeView` — an element of the
`views` prop array — among its dependencies. A host that builds that array
inline hands over a fresh element object on every one of its own renders, so
the dependency changed identity every render and a new `find()` went out each
time. Measured with an instrumented adapter and three parent re-renders: 4
queries where a hoisted array gives 1. Each extra query also re-delivered a
fresh row array to the child view, since rows go down as `data={data}`.
The effect now depends on a reference that changes only when the values it
reads change — the active view's `filter` and `sort`, plus its `id`. The card
that reported this said the effect reads `filter` and `type`; measured, the
second read is `sort`, so a fix written from that sentence would have dropped
the sort dependency.
`useStableIdentity` compares structurally and never serializes, so it stays
correct for values with no faithful stringification (a `Date`, a function, a
`Map`, `NaN`, key-order instability), and every case it cannot model resolves
to "changed" — a redundant query, never a withheld one.
Precedence is untouched: a named `listViews` config still outranks the view's
filter and sort, which still outrank `table.*` and its deprecated aliases.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011SfZeFWrhGLHmfq61xbz4q✅ Console Performance Budget
The eager closure is every chunk the entry reaches through static imports — what the browser fetches and parses before the app renders. The entry chunk on its own is a small fraction of it. 📦 Bundle Size Report
Size Limits
|
os-support-ai
commented
Aug 26, 2026
ACCEPT on the diff. ⏸ Landing held until every check is green — 18 success / 3 skipped / 8 still running, zero failures at The two constraints from the dispatch, verified against the branch rather than the reportPrecedence unflattened. Both reads still sit behind The other four dependency arrays untouched. The card's factual error is confirmed corrected. The card said the effect reads Public-surface check — and a correction to how this seat first ran it
Why the mechanism is right, not just adequateThe dispatch's one hard constraint was that the identity must stay correct for values that do not round-trip through naive stringification. The chosen comparison never serializes, so a Carrying The strongest thing in the reportThe card's own harness turned out to be vacuous in the agent's hands:
It then said so, did not chase it, and built a deterministic pin instead. The new harness reads 4-vs-1 before the fix and 1-and-1 after, where the card recorded 3-and-1 — and it explains the difference (the card's 20 ms-timer parent landed one re-render before #6419's schema gate opened) rather than quietly adopting the new number. The invariant survives both readings: before the fix each parent re-render costs exactly one extra Reverse verification predicted the direction first and observed exactly it — restoring the pre-fix dependency array reds the two churn-direction assertions ( Disclosures checked and acceptedThe +4 new lint warnings are all Generated by Claude Code |
Uh oh!
There was an error while loading. Please reload this page.
Fixes#6460
All measurements below were taken on this branch's final commit,
dbeb02d8b, whose tree is the merge baseb1a732b22plus this diff.The defect, measured
ObjectView's non-grid fetch effect listedactiveView— an element of theviewsprop array (viewsPropResolved?.find(...) || viewsPropResolved?.[0]) — among its dependencies. A host that builds that array inline produces a fresh element object on every one of its own renders, so the dependency changed identity every render and a newfind()went out each time.Measured on the merge base with
packages/plugin-view/src/__tests__/ObjectView.viewIdentityDeps.test.tsx— instrumented adapter, oneObjectView, three parent re-renders after the first query settles,defaultViewType: 'calendar':findcalls, beforeviewsbuilt inline (fresh array each render)viewshoisted (stable array) — controlThe two runs differ only in whether the array literal is hoisted, which is what makes this a defect rather than a property of re-rendering.
On the card's "3 vs 1"
The card recorded 3-and-1 post-#6419 and the dispatch order asked for that to be re-measured rather than assumed. It is 4-and-1 here, and the difference is the harness, not the code. The card drove its parent from a 20 ms timer against a 30 ms schema read, so one of its three re-renders landed before#6419's gate opened and cost no query. This harness re-renders deterministically after the first query has settled, so all three land after the gate. The invariant underneath both readings is the same and is the thing worth stating: before the fix, each parent re-render costs exactly one extra
find; after it, zero. A timing-dependent count is not a good pin, which is why the committed test does not use one.Beyond the query count:
ObjectViewhands rows to the child view asdata={data}, so each extrafind()also re-delivered a fresh row array downstream. The first test assertsdeliveriesis length 1, so that half is pinned too.The card body says the effect "only ever reads
activeView?.filterandactiveView?.type". Verified againstmainatb1a732b22: inside the effect bodyactiveViewis read at exactly two sites, and the second issort, nottype(typereaches the effect only viacurrentViewType, already its own dependency). Line numbers moved by one from the PM's dispatch-time reading — the reads are at:825and:860on the merge base, matching; after this diff they sit at:864and:899.A fix written from the card's sentence would have dropped the sort dependency, so a host changing only a view's
sortwould stop re-fetching — a worse defect than the churn, and one that passes any test authored from the card's own wording.re-fetches when only the view's SORT changesin the new test file is the pin that holds that shut.The mechanism, and why it is faithful to values that do not stringify
The effect now depends on
activeViewQueryInputs—{ id, filter, sort }read off the active view and passed throughuseStableIdentity, a new internal helper (packages/plugin-view/src/stableIdentity.ts, not exported from the package index).useStableIdentityreturns the previous reference while the value is structurally unchanged. It compares structurally and never serializes, which matters because a view'sfilterandsortare author-supplied metadata this package does not get to constrain. AJSON.stringifykey would be wrong in both directions:JSON.stringifydrops keys whose value isundefinedor a function and renders aMap/Set/class instance as{}, so{ a: undefined },{ a: () => 1 },{}and{ a: new Map() }all serialize to the same four characters;NaNandInfinityboth becomenull.{ a: 1, b: 2 }and{ b: 2, a: 1 }are one filter and two strings.Structural comparison avoids all three: key order cannot matter because nothing is serialized, a
Dateis compared by its instant, and anything the function does not model — functions,Map,Set,RegExp, class instances — falls back toObject.is, i.e. reference identity, which can never call two different values equal.The safety invariant, which is what makes this safe to sit under a data dependency: every uncertainty resolves to "not equal". An unmodelled type, a differing key count, and a structure deeper than the depth bound all return
false, which yields a new reference and therefore a re-fetch. This can only ever remove a redundant query; it can never withhold a needed one.packages/plugin-view/src/stableIdentity.test.tsasserts each stringify-collapse case as not equal, with the collapse asserted alongside it, so the tests state what they protect against rather than merely asserting a boolean.Precedence is not flattened. Both reads still sit behind
currentNamedViewConfigin the same||chains, unchanged; this decides only when the effect re-runs, never which source wins. A control test asserts a namedlistViewsconfig's filter and sort still outrank the view's.idis carried deliberately though the effect does not read it. It is a string, so it cannot churn, and pinning it keeps switching views observably re-fetching even between two views whose filter and sort happen to coincide — the acceptance control. Same ingredients as the display key this file already derives at what is now:1678:`${schema.objectName}-${activeNamedView || activeView?.id || 'default'}-${currentViewType}-${refreshKey}`, so this is convergence on in-file precedent."Ask hosts to pass a stable array" was rejected, per the ruling on the card: it is a contract change on every caller of a published component dressed as a bug fix, and it leaves the defect live for every host that does not comply.
Reverse verification
Direction predicted before running: restoring the pre-fix dependency array turns exactly the two churn-direction assertions red (expecting 1
find, observing 4) while every control stays green.Observed, with the fix committed first so the restore had a real commit to return to — mutation confirmed on disk by counting both spellings and by blob hash, restore confirmed by the blob matching
HEAD:packages/plugin-view/src/ObjectView.tsxandgit diff HEADbeing empty (the script carried atrap … EXIT INT TERM, absolute paths, and treated an empty hash as failure):No rebuild step is involved on either leg: the test imports
../ObjectViewfrom source, so the mutation reaches the run directly.Exactly the predicted two, and both controls that separate "the churn is fixed" from "the effect stopped reacting" — filter change, sort change, active-view-id change, named-view precedence, and a filter
Datemoving to a different instant — stayed green.Gates
Each verdict below is the gate's own reported line, with its exit code captured before any pipe.
pnpm exec vitest run packages/plugin-view/Test Files 26 passed (26)/Tests 254 passed (254)pnpm --filter @object-ui/plugin-view run type-check(tsc --noEmit && tsc -p tsconfig.test.json)pnpm exec eslint packages/plugin-view --no-inline-config✖ 290 problems (0 errors, 290 warnings)node scripts/check-changeset-presence.mjs✅ 1 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)grep -naP '[\x00-\x08\x0b\x0c\x0e-\x1f\x7f]'over all five touched filesType-check needed
pnpm --workspace-concurrency=2 --filter '@object-ui/plugin-view^...' buildfirst: in a fresh worktree the unbuilt dependency closure makestscreportTS2307: Cannot find module '@object-ui/components'and a cascade ofTS7006implicit-anys that vanish once the.d.tsfiles exist.The lint run is a declared narrowing, and a measured one
Repo-wide
pnpm lintis the CI job's own run. What is reported here ispackages/plugin-viewscoped, with the three pieces of evidence that make the narrowing a measurement rather than a skip:--format jsonreports 42 files linted, 0 errors, 290 warnings.parserOptions.project, noprojectServiceanywhere ineslint.config.js), so a file's verdict depends only on its own contents plus the shared config. This diff touches five files, all insidepackages/plugin-view, so it cannot move the verdict of any file outside the scoped run.Warning accounting, since this PR adds four.
eslint.config.jsdeliberately setsreact-hooks/refstowarn("codebase predates these rules") and.github/workflows/lint.ymldeliberately sets no--max-warnings, so errors are the gate. The four new warnings are all instableIdentity.ts, on the ref read/write inuseStableIdentity— the standard identity-preserving memo pattern. The access is derived purely from(previous, value), so it is idempotent and safe under StrictMode's double invocation; an inlineeslint-disablewould be inert anyway, becausepnpm lintruns with--no-inline-config.ObjectView.tsxitself adds zero warnings. Verified by linting the merge-base copy of the file against the same config:Scope
This one effect.
activeViewalso appears in the dependency arrays at:727,:1108,:1380and:1474on the merge base; they are untouched, and nothing here was swept in as a rider. No finding is filed against them — assessing whether they share this defect needs its own measurement, which this card's scope does not fund.One observation worth recording without acting on it:
currentNamedViewConfigis also in this effect's dependency array and is auseMemooverschema.listViews, so a host that inlines itsschemaobject can churn this effect through that door instead. That is out of this card's stated scope (the ruling is aboutactiveView) and is unmeasured here, so it is noted rather than filed or fixed.No docs change:
useStableIdentityis internal and not exported from the package index, and the fix changes no authorable surface — the changeset is this PR's release-notes input.Generated by Claude Code
Generated by Claude Code