fix(core): ValueDataSource clones inline rows with structuredClone, not a JSON round-trip - #9192
Conversation
…N round-trip
`ValueDataSource` deep-cloned its inline rows through
`JSON.parse(JSON.stringify(...))` in the constructor and again in `getAll()`.
The clone exists only to stop external mutation of a read-only query source —
an aliasing barrier — but the round-trip also made it a serialization
boundary, which nothing asked for.
Consequence: every row routed through `provider: 'value'` silently had to
survive `JSON.stringify`. A `Date` came back as a string, keys whose value was
`undefined` disappeared, `Map`/`Set` flattened to `{}`, `NaN`/`Infinity`
became `null`, and a `BigInt` or a cyclic record graph threw. `ViewData.items`
is `z.array(z.unknown())` in `@objectstack/spec`, and objectui#6018 pinned the
guarantee in words — an inline value never has to be serializable at all.
`structuredClone` is the same aliasing barrier without the serialization
boundary. It still throws `DataCloneError` on a function or a DOM node, and
that stays loud: no try/catch fallback, since falling back to the round-trip
would restore exactly the silent flattening this replaces.
Maintainer ruling A on objectui#9061 (2026-09-11, decision batch #115).
Part of objectui#9175
Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
…cument it `filter-dialect-equivalence-7221.test.ts` pinned `NaN` as EXCLUDED by `is_not_null`, and its own docblock already said why that was not the operator's doing: the constructor's JSON round-trip wrote `NaN` as `null`, so by the time any filter ran the row genuinely held `null`. With the round-trip gone, `NaN` reaches the matcher as `NaN` — neither `null` nor `undefined` — and the PRESENCE operator keeps it. Re-measured rather than re-based: the file's own instruction. Both dialect equivalence contracts stay green and untouched; only the row-set constant and the comment that explains it move, and the old set is kept in the comment as the evidence the exclusion was an artefact of the clone. Also documents the clone in the adapters README and adds the changeset. Part of objectui#9175 Co-authored-by: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ
✅ 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
|
PM review — ACCEPT on content; ⛔ arm HELD until CI convergesRead against The executable change ✅Both round-trips are gone and nothing else moved: The premise correction — independently verified, and it is the most valuable thing in this deliveryThe card's Acceptance bullet said the three red assertions on PR #9138 would pass with that diff untouched. The report says only one of the three is a clone problem. I did not take that on the ablation's word — I read the fixtures at source on // ObjectMap.filterConfig.test.tsx
await renderMap({
type: 'object-map',
data: { provider: 'value', items: ROWS },
map: DECLARED_MAP,
filter: [['owner', '=', 'me']],
});
expect(markers).toHaveLength(1);Once PR objectui#9138 makes the inline arm honour ⇒ The pin edit — this is the hazard, and it was handled correctly ✅
That is a re-measurement with its reasoning on the page, not a pin re-based to go green. Source-text pin sweep — with a correction to the report's version of itThe report swept for pins reading The conclusion survives, measured rather than assumed: the diff across all three files adds and removes zero ⭐ An instrument note I owe from my own run of that sweep: my first pass was Clause-②
⛔ Why the arm is held36 check runs, 4 still running, 0 failing. Not-red-yet is not green — that discipline was bought with objectui#9169, where I armed on a partial read and On this card's fate after landingEverything the maintainer's ruling A required is delivered. The single unmet acceptance bullet is not unfinished work — it is measurably unachievable by any clone change, and its two remaining fixtures belong to objectui#9061 / PR objectui#9138's repair. So on landing I will strip the dispatch labels, record the correction on objectui#9061 and PR objectui#9138 where the two fixtures actually live, and close objectui#9175 as delivered rather than leaving it open against a bullet that was wrong when it was written. Generated by Claude Code |
Part of objectui#9175. Deliberately not a closing keyword: the ruled change is complete, but one bullet of that card's Acceptance turns out to be measurably unreachable from this card, and the ablation below is the evidence. A maintainer should decide the fate of that card knowingly, rather than have a merge decide it — hence no closing keyword anywhere in this body.
Executes maintainer ruling A on objectui#9061 (2026-09-11, decision batch #115).
What changed
ValueDataSourcedeep-cloned its inline rows withJSON.parse(JSON.stringify(...))— in the constructor and again ingetAll(). Both are nowstructuredClone.The clone exists for one reason, stated in the comment above it: "Deep clone to prevent external mutation". That is an aliasing barrier on a read-only query source. The round-trip was an aliasing barrier too, but it was also a serialization boundary — and nothing asked for one. So every row that reached
provider: 'value'silently acquired a requirement the contract never states:ViewData.itemsisz.array(z.unknown())in@objectstack/spec, and objectui#6018 pinned the consequence in words — an inline value never has to be serializable at all.⛔ No
try/catchfallback. A function-valued row now throwsDataCloneErrorat construction, loudly, and that is the intended shape: a fallback would restore exactly the silent flattening this replaces.Measurement first
The two sites, located by text (not by the card's line numbers)
The card measured
~:1077/~:1318and told the dev to re-locate. Onorigin/mainatb17e5e397they are:packages/core/src/adapters/ValueDataSource.ts:1055— the constructor.packages/core/src/adapters/ValueDataSource.ts:1296— the snapshot accessor. It is spelledgetAll(), not a getter.Consumer census
git grep -l "ValueDataSource", no pathspec glob: 79 tracked files.ValueDataSourceZzq: 0 files, exit 1.git grep -l ... -- 'packages/*/src' 'apps/*/src' examples e2ereturned 1 file where the un-globbed query returns 102. Every number here is from the un-globbed form.Three production construction sites in the tree, and no others:
itemspackages/core/src/adapters/resolveDataSource.ts:70viewData.items, forprovider: 'value'packages/plugin-designer/src/FieldDesigner.tsx:152filteredFields— in-memory field metadatapackages/plugin-designer/src/ObjectManager.tsx:101displayObjects— in-memory object metadataresolveDataSourceis reached from exactly two places —useViewData(packages/react/src/hooks/useViewData.ts:109) andObjectGantt(packages/plugin-gantt/src/ObjectGantt.tsx:624) — plus the inline arms objectui#9061 / objectui#9136 add. Row readers:find(), which does not re-clone ([...this.items]is a shallow array copy, so rows are the constructor's clones), andgetAll(), which has no production reader anywhere in the tree.Per consumer: what it can carry, and whether it depends on the flattening
Date/undefined/Map/Set/BigInt/ function?resolveDataSource→useViewData,ObjectGantt, and the objectui#9061 / objectui#9136 inline armsitemsisz.array(z.unknown()); a host that inlines already-resolved rows hands over whatever it holds —Dates,undefinedkeys, back-referencesFieldDesigner,ObjectManagerBoth routes are pinned in
packages/core/src/adapters/__tests__/ValueDataSource.structuredClone-9175.test.ts, each with the shape it receives today and the shape it receives after.One dependence on the flattening was found, and it is a pin, not a production consumer — reported rather than papered over:
No production site depends on a
Datearriving as a string or on anundefinedkey vanishing. Both were searched for across the un-globbed census.Two observable consequences worth naming
Both are through the adapter's own API, not only in the rows, and both are pinned:
getObjectSchemainfers withtypeof, so aDatecolumn infers'object'where it inferred'string', and a key whose value isundefinednow appears in the inferred schema at all.$orderbyon aDatecolumn sorts chronologically instead of lexically over ISO text — the same order for ISO-8601, a different one for any other rendering.Runtime target — the card's second stop condition
Not triggered. Measured, not assumed:
@object-ui/coredeclares noengines; the root declaresnode >=22.11.structuredCloneis a Node global since 17.0.browserslistanywhere in the repo —git grep browserslistoutside the lockfile returns nothing.target: "ES2020"intsconfig.base.jsonis a syntax target.structuredCloneis a host global, untouched by downlevelling.packages/app-shell/src/views/metadata-admin/EmbeddedItemEditor.tsx:272andpackages/plugin-designer/src/hooks/useClipboard.ts:30.@object-ui/coreitself already calls it, guarded, atpackages/core/src/utils/freeze-schema.ts:211.So: no polyfill, and no new runtime dependency.
Source-text pin census
git grep -l "readFileSync"over the test corpus: 223 files. Lit control lights as specified — the query hitspackages/i18n/src/__tests__/residue-namespaces-3546.test.tsx, which namespackages/plugin-kanban/src/KanbanImpl.tsx.No test or script reads
ValueDataSource.tsas source text. The only two files naming it by path do so in prose comments. Widening the scope toscripts/(211 morereadFileSyncfiles) surfaced one real registry that does read this file's bytes:scripts/dollar-dialect-alias-census.mjsscans every tracked file for$-dialect operator spellings and classifiespackages/core/src/adapters/asrefusal-ledger. This diff adds and removes no operator spelling, so its census is unmoved — but it is the registry the widening was for, and it is named here rather than left unfound.Ablation — and the one acceptance bullet it falsifies
Measured on a throwaway local merge of this branch with PR objectui#9138's head (
7bd112303), created in its own worktree and never pushed. PR objectui#9138's own files came out byte-identical to its head (agit diffover its eight owned paths is empty); the ninth,content/docs/plugins/plugin-calendar.mdx, merged with an unrelatedmainedit and all ten of the PR's added lines are present.Both legs run the same two named files (17 tests):
ValueDataSourcestructuredClone(this branch)Leg 2 reproduces exactly the three standing reds PR objectui#9138 names. Leg 1 turns one of them green:
packages/plugin-map/src/ObjectMap.dataConfigMemo.test.tsx— "renders inline data the serializer cannot handle — identity needs no round-trip" (objectui#6018). The one PR objectui#9138 calls "the decision". Red under leg 2, green under leg 1.packages/plugin-map/src/ObjectMap.filterConfig.test.tsx— "really is the DEFAULT config that applies, not 'no config at all'". Red under both legs.packages/plugin-map/src/ObjectMap.filterConfig.test.tsx— "appliesschema.mapwhen an array filter is authored alongside it". Red under both legs.PR objectui#9138's own two new suites (
ObjectMap.inlineQueryKeys-9061,ObjectCalendar.inlineQueryKeys-9061, 17 tests) are green on the combined tree.schema.filteron the inline arm: case 2 authorsfilter: { map: {...} }against a row that has nomapfield, case 3 authorsfilter: [['owner', '=', 'me']]against a row that has noowner. Neither expectation touches serialization. PR objectui#9138's own body already classifies them correctly — "ordinary fixture triage … NOT a second decision" — it only carried them into option A as well, and objectui#9175 inherited that sentence verbatim.⛔ They are not re-based here. objectui#9175 forbids it, and they are not this card's fixtures — they belong with objectui#9061's own repair.
Both ablation legs proved themselves on disk before anything was read. Mutation: the new text went 1 → 0 and the old text 0 → 1 at both anchors, blob
ab57948…→2b551d4…. Restore:git checkout HEAD -- ...under anEXIT INT TERMtrap with an absolute repo root, verified by an emptygit diff HEADand the blob back at the HEAD blob — never by an exit code. The measurement worktree and its branch are gone.Fixture triage — one file, re-measured rather than re-based
packages/core/src/utils/__tests__/filter-dialect-equivalence-7221.test.tsinstructs its own reader: "A case going red means the measured behaviour moved — re-measure before changing the expectation." Done:NaNis now KEPT byis_not_null. It is neithernullnorundefined, and the operator asks about presence — so the new row set is the operator's own answer, and the old one was an artefact of the clone the file itself documented.PRESENT_EDGE_IDSconstant and the comment explaining it move, and the old set is kept in that comment as the evidence.undefinedvalue's key, which is why theundefinedrow and themissing-keyrow were indistinguishable. They are now distinguishable rows, andis_not_nullanswers NO to both — which is what the operator is for.Verification
Run from the repo root; exit codes captured by redirection before any pipe.
1 passed (1)/18 passed (18)packages/core/+ 18 downstream files touchingValueDataSourceorprovider: 'value'170 passed (170)/3537 passed (3537)60 passed (60)/683 passed (683)pnpm --filter @object-ui/core... builddist/adapters/ValueDataSource.jscarriesstructuredCloneand no round-trippnpm --filter @object-ui/core type-checktsc -p tsconfig.test.json --listFilesconfirms the new test file is IN that programpnpm --filter @object-ui/core lintanywarnings, 0 errors)check:control-bytes,check:changeset-claims,check:new-line-citations,check:doc-fences,check:shell-escape-residuecheck-changeset-presence,check-changeset-no-majorcheck:governed-queue-guard --testover all five changed pathsNOT GOVERNEDThe affected-file list was rebuilt without pathspec globs after the false-zero above: 102 test files name
ValueDataSourceorprovider: 'value', and all 102 were run.NOT MEASURED:
check:readme-exports. It requiresturbo run build --filter='./packages/*'across every package before it can read a single declared type entry — a farm-scale prerequisite its own dedicated workflow performs. Locally it reports534 self-import(s) could not be judged … not on disk, which is the prerequisite, not a red. The README edit here adds zero import lines and zero fenced blocks (measured on the diff), so it cannot move that gate's population.Clause-②:
yesA published adapter's observable behaviour changes — a
Datenow survives as aDate, and a function-valued row now throws where it was silently stripped.needs:contract-reviewis on this PR.⛔ This PR stays draft: no ready flip, no merge queue, no auto-merge.
Changeset:
@object-ui/coreminor — a widening (more shapes reach the renderer as authored) with one deliberate narrowing (the loud refusal).minorrather thanmajorper this repo's version-alignment rule; the breaking semantics are spelled out as a before/after table in the changeset body. The breakage is the cost of the repair, not its purpose: the purpose is to stop imposing a serialization contract nobody declared, and the loudDataCloneErroris what honesty about that costs.Acceptance notes
packages/core/src/utils/freeze-schema.ts:211still guardsstructuredClonebehindtypeof structuredClone === 'function'and falls back to a JSON round-trip. Given the unguarded calls already shipping in@object-ui/app-shelland@object-ui/plugin-designer, that guard is dead weight and its fallback silently reintroduces flattening on a runtime that would already have failed elsewhere. Out of scope here (objectui#9175 is one adapter, and touchingcloneAsOverridewould widen the contract-review surface). Successor: whoever next editsfreeze-schema.ts.getAll()has no production reader in the tree. Dead-code observation only; ⛔ not a finding class this card files.Authored by Claude Code for the ObjectUI dev seat; the originating session is
https://claude.ai/code/session_01UzHd6hDYatoDn17BuwKxnZ.Generated by Claude Code