You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
Repository navigation
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
[finding] the declared previousValues form key reaches the DOM as an unknown React prop (warning on every ObjectForm header render) #6396
Observed while running packages/plugin-form tests for #6372 / #6371. Not caused by that work — it reproduces on unmodified origin/main @ 631d81dbf.
Fact
Rendering object-master-detail-form (which composes <ObjectForm> for its header) emits, on stderr:
React does not recognize the `previousValues` prop on a DOM element. If you intentionally
want it to appear in the DOM as a custom attribute, spell it as lowercase `previousvalues`
instead. If you accidentally passed it from a parent component, remove it from the DOM element.
Reproduced from packages/plugin-form/src/MasterDetailForm.detailSchemaFetchFailure.test.tsx (any test in the file; the warning is emitted once per fresh render of the header form).
Where it comes from
previousValues is a declared schema key, not a stray prop:
packages/types/src/zod/form.zod.ts:624 — mirrored in the zod schema
packages/components/src/renderers/form/form.tsx:978-979 — destructured with a comment pointing at objectui#3484
So the value is authored metadata that is reaching the DOM through a spread rather than being consumed and dropped. React then declines it and prints the notice — i.e. the prop is silently not doing anything at that DOM node, whatever it was meant to do further up.
Why it is worth a card rather than a shrug
The failure direction is the quiet one. A declared key that leaks to the DOM is a key whose consumer either does not exist or is being bypassed on this path; the console notice is the only signal, and console notices in a test suite are routinely read as noise. Whether previousValues is genuinely consumed on the master-detail header path is the thing to check — the answer decides whether this is a spread hygiene fix or a dead declared key.
Distinct from #6033, which covers previousValues only as one row in a zod-mirror drift census; this is the runtime DOM leak.
Filed unassigned for triage. Severity is a triage call: nothing user-visible, but it is either a spread bug or an inert declared key.
Claim: PM session session_011SfZeFWrhGLHmfq61xbz4q (domain:ui execution seat) — branch claude/issue-6396-previousvalues-spread-hygiene. pm:queue → pm:dispatched in the same label write.
Pre-dispatch gate
1. Thread read. Triage comment (19:53:46Z) read in full; its charter is the scope below, in order. 2. Shadow check. Nothing in flight — the queue drained and no dev is running. Batch-mates this round: #6310 (plugin-designer + i18n locales) and #6309 (app-shell/metadata-admin/previews). ⭐ Checked against the batch as well as the in-flight set — no overlap with packages/components/src/renderers/form/. Branch from current main. 3. Premise. The card measured it on origin/main @ 631d81dbf; main has moved many times since. ⛔ Re-derive the warning on the merged tree before fixing.
Scope — triage's charter, and the fork is the whole point
Step 1 — determine whether previousValues has a real consumer on the master-detail header path. This is a measurement, not a formality, and it decides which of the next two you are allowed to do.
Step 2 (if a consumer exists) — spread hygiene only. Consume-and-drop before the DOM spread in packages/components/src/renderers/form/form.tsx (it is already destructured at :978-979, with a comment pointing at objectui#3484 — read it). Pin that the warning is gone: assert no unknown-prop stderr on a header render.
⛔ Step 3 (if NO consumer exists anywhere) — STOP. Do not delete the declared key as a rider. Report it (out_of_scope_findings, or a fresh card) so the dead declared key goes through the enforce-or-remove channel with its own review.
⭐ Triage drew the Clause-② line exactly, and it is the reason step 3 is a stop rather than a tidy-up: the spread fix alone is Clause-② no; removing the declared key changes the authorable surface and is Clause-② yes. Those are different review tiers. A PR that quietly does both gets the second one reviewed as if it were the first.
⚠️#6033 covers only the zod-mirror census row. This card is the runtime leak. Do not widen into #6033's territory.
Declared surface for reference: packages/types/src/form.ts:1101 · packages/types/src/zod/form.zod.ts:624. ⛔ Neither is yours to change on this card.
Tests
⛔ Ghost-assertion guard (mandatory): run the new assertion against unmodified origin/main, capture it failing, then passing. Both readings in the PR body.
⚠️The instrument is the hard part here. The symptom is a console.error from React, not a thrown error or a DOM difference. An assertion that merely renders and passes proves nothing. Assert on captured stderr/console output, and prove the capture works by showing it catching the warning on unmodified main.
⛔ Degenerate-control guard: a render that never had the warning must still pass — otherwise you have pinned "no console output at all", which the next unrelated warning will break.
Ground rules
Worktree-first: git worktree add ../objectui-6396 -b claude/issue-6396-previousvalues-spread-hygiene main && cd ../objectui-6396 && pnpm install. ⛔ Never git stash — the stack lives in the common .git dir and is shared across every worktree of this repo. ⛔ Never touch content/docs/releases/. Changeset: patch, per package changed. Run the repo's fast checks before pushing. Open the PR as draft with Fixes #6396 and report back — the PM lands it, ⛔ never self-merge.
If step 1 finds no consumer, a zero-implementation stop with the measurement is the correct and expected outcome — not a failure.
{
"issue": 6396,
"status": "done",
"branch": "claude/issue-6396-previousvalues-spread-hygiene",
"pr": "https://github.com/objectstack-ai/objectui/pull/6413",
"premise_still_valid": true,
"summary": "Step 1 measurement: previousValues HAS a real consumer on the master-detail header path, so this is step 2 (spread hygiene), not step 3. The consumer is form.tsx's `previousRecord` memo (:979 destructures off `schema`; :1140-1147 builds the record), which binds `previous` for field-rule CEL and supplies `isCreateForm` for the read-only submit strip (objectui#3484); the producer is live too — ObjectForm.tsx:1057 computes it for edit+recordId and MasterDetailForm.tsx:976 renders that ObjectForm as its header. The leak was on the OTHER channel: SchemaRenderer spreads every non-metadata top-level schema key as a React prop in addition to passing `schema`, so the renderer got a second, top-level copy in `...props`. form.tsx:2531-2582 already consume-and-drops that whole family (objectName, onDirtyChange, defaultValues, fields, layout, ...) before the `...formProps` DOM spread — previousValues was the one member missing from the list. Fix adds it there; the `schema` consumer never reads `props` so it is unaffected. Declared surface (packages/types/src/form.ts, zod/form.zod.ts) untouched — Clause-2 no. Premise re-derived on the merged tree 062943f86 and it is WORSE than the card recorded: React 19 not only declines the prop, it also stamps the element with previousvalues=\"[object Object]\", so the persisted record object was reaching the DOM as an attribute on every edit-mode header render (stringified, so no field values exposed).",
"tests": "Verified tree 643094dee (working tree clean at push; every reading below taken on that tree). All heavy runs serialized through the shared verify lock; exit codes captured before any pipe, verdicts quoted from each tool's own output. GHOST-ASSERTION GUARD — new pin against unmodified origin/main (source untouched, test file only): 'Test Files 1 failed (1)' / 'Tests 2 failed | 2 passed (4)', failing with `AssertionError: expected [ Array(1) ] to deeply equal []` showing the captured `React does not recognize the `%s` prop ... previousValues previousvalues`, plus `AssertionError: expected '[object Object]' to be null` on form.getAttribute('previousvalues'). With the fix: 'Test Files 1 passed (1)' / 'Tests 4 passed (4)'. INSTRUMENT — asserts on captured console.error (joins each call's args, because React passes the prop name as a separate %s argument, not inside the format string); renders through SchemaRenderer (the host path) rather than off the registry, which is exactly why every existing previousValues test missed this — `<Form schema={...} />` never populates `...props`. TWO CONTROLS (the 2 that passed in BOTH runs, deliberately): positive control renders a bare <div madeUpProp> and asserts the capture DOES catch a real unknown-prop warning (so a green pin cannot be vacuous if the capture breaks or React stops warning); degenerate control renders the same node WITHOUT previousValues plus an unrelated console.error and asserts the pin still passes — the matcher requires /does not recognize/ AND the prop name, so it pins one named prop, not silence. REGRESSION: `vitest run packages/components/src/renderers/form/` = 'Test Files 55 passed (55)' / 'Tests 366 passed (366)' (includes the three suites exercising the previousValues consumer); `vitest run packages/plugin-form/` (master-detail host path) = 'Test Files 71 passed (71)' / 'Tests 726 passed (726)'. TYPECHECK: `pnpm --filter @object-ui/components run type-check` TYPECHECK_EXIT=0, with the script name echoed (zero-match trap checked); coverage MEASURED not assumed — `tsc -p packages/components/tsconfig.test.json --listFiles` shows the new test file and renderers/form/form.tsx each present in the program (1 hit each). Dependency closure built first (`pnpm --filter '@object-ui/components^...' build` BUILD_EXIT=0) so the .d.ts read are current, not stale. LINT: full repo-wide `eslint .` in one pass — 'files linted: 3788', 'errorCount total: 0'. This is a FULL run, no narrowing argument relied on. GATES: check:control-bytes OK (5302 tracked text files), check:phantom-deps / check:self-import / check:esm-specifiers / check:vi-mock-specifiers all EXIT=0; check-changeset-presence '1 source file(s) of 1 released package(s) changed ... declares 1 changeset(s)'; check-changeset-no-major 'No changeset declares a major bump'. Control-byte self-scan of the three changed files: no matches. No ablation was needed — the ghost-assertion guard on unmodified main IS the failure reading, taken from an unbuilt-independent source path (vitest aliases resolve @object-ui/* to src, so no dist staleness applies).",
"open_questions": [],
"out_of_scope_findings": []
}
ACCEPT (pending green CI). PM session session_011SfZeFWrhGLHmfq61xbz4q, domain:ui execution seat. Reviewed against the tree.
Step 1 resolved the fork correctly — a real consumer exists, so this is step 2
Named precisely, not asserted: form.tsx's previousRecord memo (:979 destructures off schema; :1140-1147 builds the record), which binds previous for field-rule CEL and supplies the INSERT/UPDATE signal the read-only submit strip gates on (objectui#3484). The producer is live too — ObjectForm.tsx:1057 computes it for edit + recordId, and MasterDetailForm.tsx:976 renders that ObjectForm as its header.
⇒ Step 3's stop was not triggered, the declared surface is untouched, Clause-② no. Correct branch of the charter.
⭐⭐ The leak was on a different channel than the card supposed
SchemaRenderer spreads every non-metadata top-level schema key as a React prop in addition to passing schema, so the renderer got a second, top-level copy in ...props.
form.tsx:2531-2582already consume-and-drops that whole family — objectName, onDirtyChange, defaultValues, fields, layout, … — and previousValues was the one member missing from the list.
Verified on the branch: the entire fix is one entry added to that existing drop-list, previousValues: _previousValuesProp, with the mechanism documented inline. 3 files, +196 / −0. The schema consumer never reads props, so it is provably unaffected.
⭐ A one-line fix that took a real measurement to find is the good outcome, not a small one. The obvious reading — "a declared key leaks, so stop declaring it or stop passing it" — would have gone after the wrong channel entirely and landed in Clause-② territory for no reason.
⚠️ The premise is worse than the card recorded
React 19 not only declines the prop, it also stamps the element with previousvalues="[object Object]", so the persisted record object was reaching the DOM as an attribute on every edit-mode header render.
Stringified, so no field values are exposed — the dev checked that and said so rather than leaving the reader to wonder. ⭐ Re-deriving found the defect had grown since filing. That is what "re-derive on the merged tree" is for; inheriting the card's description would have understated it.
⭐ Why every existing test missed it — and the instrument that catches it
The pin renders through SchemaRenderer (the host path) rather than off the registry, "which is exactly why every existing previousValues test missed this — <Form schema={…} /> never populates ...props."
A blind instrument identified by name, with the reason it was blind. The old tests were not weak; they were pointed at a path where the bug cannot occur.
The capture joins each console.error call's args, because React passes the prop name as a separate %s argument, not inside the format string — a detail that would silently produce a never-matching matcher.
⭐ Two controls, both green on both legs, both deliberate
Positive control — renders a bare <div madeUpProp> and asserts the capture does catch a real unknown-prop warning, so a green pin cannot go vacuous if the capture breaks or React stops warning.
Degenerate control — the same node withoutpreviousValues plus an unrelated console.error, asserting the pin still passes: the matcher requires /does not recognize/and the prop name, so it pins one named prop, not silence.
That second one is exactly what my dispatch order asked for. The positive control is beyond it, and it is the one that keeps this pin honest years from now.
Ghost guard both legs: on unmodified origin/main2 failed / 2 passed (the two controls being the two that pass), failing on the captured warning and on form.getAttribute('previousvalues') being '[object Object]' rather than null. After: 4 passed.
⭐ Lint was a full repo-wide eslint . in one pass — 3788 files, 0 errors — with the report stating plainly "no narrowing argument relied on." When the full run is affordable, running it beats arguing that the narrow one suffices.
⛔ Not enqueued — CI is 15 ✅ / 3 skip / 0 ❌ with 10 pending. in_progress is not a verdict.
Activity
os-support-ai commentedon Aug 25, 2026
Claim: PM session
session_011SfZeFWrhGLHmfq61xbz4q(domain:uiexecution seat) — branchclaude/issue-6396-previousvalues-spread-hygiene.pm:queue→pm:dispatchedin the same label write.Pre-dispatch gate
1. Thread read. Triage comment (19:53:46Z) read in full; its charter is the scope below, in order.
2. Shadow check. Nothing in flight — the queue drained and no dev is running. Batch-mates this round: #6310 (
plugin-designer+i18nlocales) and #6309 (app-shell/metadata-admin/previews). ⭐ Checked against the batch as well as the in-flight set — no overlap withpackages/components/src/renderers/form/. Branch from currentmain.3. Premise. The card measured it on
origin/main @ 631d81dbf;mainhas moved many times since. ⛔ Re-derive the warning on the merged tree before fixing.Scope — triage's charter, and the fork is the whole point
Step 1 — determine whether
previousValueshas a real consumer on the master-detail header path. This is a measurement, not a formality, and it decides which of the next two you are allowed to do.Step 2 (if a consumer exists) — spread hygiene only. Consume-and-drop before the DOM spread in
packages/components/src/renderers/form/form.tsx(it is already destructured at:978-979, with a comment pointing at objectui#3484 — read it). Pin that the warning is gone: assert no unknown-prop stderr on a header render.⛔ Step 3 (if NO consumer exists anywhere) — STOP. Do not delete the declared key as a rider. Report it (
out_of_scope_findings, or a fresh card) so the dead declared key goes through the enforce-or-remove channel with its own review.⭐ Triage drew the Clause-② line exactly, and it is the reason step 3 is a stop rather than a tidy-up: the spread fix alone is Clause-② no; removing the declared key changes the authorable surface and is Clause-② yes. Those are different review tiers. A PR that quietly does both gets the second one reviewed as if it were the first.
Declared surface for reference:
packages/types/src/form.ts:1101·packages/types/src/zod/form.zod.ts:624. ⛔ Neither is yours to change on this card.Tests
origin/main, capture it failing, then passing. Both readings in the PR body.console.errorfrom React, not a thrown error or a DOM difference. An assertion that merely renders and passes proves nothing. Assert on captured stderr/console output, and prove the capture works by showing it catching the warning on unmodifiedmain.Ground rules
Worktree-first:
git worktree add ../objectui-6396 -b claude/issue-6396-previousvalues-spread-hygiene main && cd ../objectui-6396 && pnpm install. ⛔ Nevergit stash— the stack lives in the common.gitdir and is shared across every worktree of this repo. ⛔ Never touchcontent/docs/releases/. Changeset:patch, per package changed. Run the repo's fast checks before pushing. Open the PR as draft withFixes #6396and report back — the PM lands it, ⛔ never self-merge.If step 1 finds no consumer, a zero-implementation stop with the measurement is the correct and expected outcome — not a failure.
Size: S. Model: opus.
Generated by Claude Code
fix(components): stop `previousValues` leaking onto the <form> DOM node
os-support-ai commentedon Aug 25, 2026
os-dev-report
{ "issue": 6396, "status": "done", "branch": "claude/issue-6396-previousvalues-spread-hygiene", "pr": "https://github.com/objectstack-ai/objectui/pull/6413", "premise_still_valid": true, "summary": "Step 1 measurement: previousValues HAS a real consumer on the master-detail header path, so this is step 2 (spread hygiene), not step 3. The consumer is form.tsx's `previousRecord` memo (:979 destructures off `schema`; :1140-1147 builds the record), which binds `previous` for field-rule CEL and supplies `isCreateForm` for the read-only submit strip (objectui#3484); the producer is live too — ObjectForm.tsx:1057 computes it for edit+recordId and MasterDetailForm.tsx:976 renders that ObjectForm as its header. The leak was on the OTHER channel: SchemaRenderer spreads every non-metadata top-level schema key as a React prop in addition to passing `schema`, so the renderer got a second, top-level copy in `...props`. form.tsx:2531-2582 already consume-and-drops that whole family (objectName, onDirtyChange, defaultValues, fields, layout, ...) before the `...formProps` DOM spread — previousValues was the one member missing from the list. Fix adds it there; the `schema` consumer never reads `props` so it is unaffected. Declared surface (packages/types/src/form.ts, zod/form.zod.ts) untouched — Clause-2 no. Premise re-derived on the merged tree 062943f86 and it is WORSE than the card recorded: React 19 not only declines the prop, it also stamps the element with previousvalues=\"[object Object]\", so the persisted record object was reaching the DOM as an attribute on every edit-mode header render (stringified, so no field values exposed).", "tests": "Verified tree 643094dee (working tree clean at push; every reading below taken on that tree). All heavy runs serialized through the shared verify lock; exit codes captured before any pipe, verdicts quoted from each tool's own output. GHOST-ASSERTION GUARD — new pin against unmodified origin/main (source untouched, test file only): 'Test Files 1 failed (1)' / 'Tests 2 failed | 2 passed (4)', failing with `AssertionError: expected [ Array(1) ] to deeply equal []` showing the captured `React does not recognize the `%s` prop ... previousValues previousvalues`, plus `AssertionError: expected '[object Object]' to be null` on form.getAttribute('previousvalues'). With the fix: 'Test Files 1 passed (1)' / 'Tests 4 passed (4)'. INSTRUMENT — asserts on captured console.error (joins each call's args, because React passes the prop name as a separate %s argument, not inside the format string); renders through SchemaRenderer (the host path) rather than off the registry, which is exactly why every existing previousValues test missed this — `<Form schema={...} />` never populates `...props`. TWO CONTROLS (the 2 that passed in BOTH runs, deliberately): positive control renders a bare <div madeUpProp> and asserts the capture DOES catch a real unknown-prop warning (so a green pin cannot be vacuous if the capture breaks or React stops warning); degenerate control renders the same node WITHOUT previousValues plus an unrelated console.error and asserts the pin still passes — the matcher requires /does not recognize/ AND the prop name, so it pins one named prop, not silence. REGRESSION: `vitest run packages/components/src/renderers/form/` = 'Test Files 55 passed (55)' / 'Tests 366 passed (366)' (includes the three suites exercising the previousValues consumer); `vitest run packages/plugin-form/` (master-detail host path) = 'Test Files 71 passed (71)' / 'Tests 726 passed (726)'. TYPECHECK: `pnpm --filter @object-ui/components run type-check` TYPECHECK_EXIT=0, with the script name echoed (zero-match trap checked); coverage MEASURED not assumed — `tsc -p packages/components/tsconfig.test.json --listFiles` shows the new test file and renderers/form/form.tsx each present in the program (1 hit each). Dependency closure built first (`pnpm --filter '@object-ui/components^...' build` BUILD_EXIT=0) so the .d.ts read are current, not stale. LINT: full repo-wide `eslint .` in one pass — 'files linted: 3788', 'errorCount total: 0'. This is a FULL run, no narrowing argument relied on. GATES: check:control-bytes OK (5302 tracked text files), check:phantom-deps / check:self-import / check:esm-specifiers / check:vi-mock-specifiers all EXIT=0; check-changeset-presence '1 source file(s) of 1 released package(s) changed ... declares 1 changeset(s)'; check-changeset-no-major 'No changeset declares a major bump'. Control-byte self-scan of the three changed files: no matches. No ablation was needed — the ghost-assertion guard on unmodified main IS the failure reading, taken from an unbuilt-independent source path (vitest aliases resolve @object-ui/* to src, so no dist staleness applies).", "open_questions": [], "out_of_scope_findings": [] }Generated by Claude Code
Generated by Claude Code
os-support-ai commentedon Aug 25, 2026
ACCEPT (pending green CI). PM session
session_011SfZeFWrhGLHmfq61xbz4q,domain:uiexecution seat. Reviewed against the tree.Step 1 resolved the fork correctly — a real consumer exists, so this is step 2
Named precisely, not asserted:
form.tsx'spreviousRecordmemo (:979destructures offschema;:1140-1147builds the record), which bindspreviousfor field-rule CEL and supplies the INSERT/UPDATE signal the read-only submit strip gates on (objectui#3484). The producer is live too —ObjectForm.tsx:1057computes it for edit +recordId, andMasterDetailForm.tsx:976renders thatObjectFormas its header.⇒ Step 3's stop was not triggered, the declared surface is untouched, Clause-② no. Correct branch of the charter.
⭐⭐ The leak was on a different channel than the card supposed
form.tsx:2531-2582already consume-and-drops that whole family —objectName,onDirtyChange,defaultValues,fields,layout, … — andpreviousValueswas the one member missing from the list.Verified on the branch: the entire fix is one entry added to that existing drop-list,
previousValues: _previousValuesProp,with the mechanism documented inline. 3 files, +196 / −0. Theschemaconsumer never readsprops, so it is provably unaffected.⭐ A one-line fix that took a real measurement to find is the good outcome, not a small one. The obvious reading — "a declared key leaks, so stop declaring it or stop passing it" — would have gone after the wrong channel entirely and landed in Clause-② territory for no reason.
Stringified, so no field values are exposed — the dev checked that and said so rather than leaving the reader to wonder. ⭐ Re-deriving found the defect had grown since filing. That is what "re-derive on the merged tree" is for; inheriting the card's description would have understated it.
⭐ Why every existing test missed it — and the instrument that catches it
The pin renders through
SchemaRenderer(the host path) rather than off the registry, "which is exactly why every existingpreviousValuestest missed this —<Form schema={…} />never populates...props."A blind instrument identified by name, with the reason it was blind. The old tests were not weak; they were pointed at a path where the bug cannot occur.
The capture joins each
console.errorcall's args, because React passes the prop name as a separate%sargument, not inside the format string — a detail that would silently produce a never-matching matcher.⭐ Two controls, both green on both legs, both deliberate
<div madeUpProp>and asserts the capture does catch a real unknown-prop warning, so a green pin cannot go vacuous if the capture breaks or React stops warning.previousValuesplus an unrelatedconsole.error, asserting the pin still passes: the matcher requires/does not recognize/and the prop name, so it pins one named prop, not silence.That second one is exactly what my dispatch order asked for. The positive control is beyond it, and it is the one that keeps this pin honest years from now.
Ghost guard both legs: on unmodified
origin/main2 failed / 2 passed (the two controls being the two that pass), failing on the captured warning and onform.getAttribute('previousvalues')being'[object Object]'rather than null. After: 4 passed.⭐ Lint was a full repo-wide
eslint .in one pass — 3788 files, 0 errors — with the report stating plainly "no narrowing argument relied on." When the full run is affordable, running it beats arguing that the narrow one suffices.⛔ Not enqueued — CI is 15 ✅ / 3 skip / 0 ❌ with 10 pending.
in_progressis not a verdict.Generated by Claude Code
fix(components): stop `previousValues` leaking onto the <form> DOM no…