Uh oh!
There was an error while loading. Please reload this page.
metadata-admin: FormFieldSpec declares dependsOn, one declaration for both halves - #5537
Merged
Merged
Conversation
…or both halves (objectui#5040)
`FormFieldSpec` is the authoring surface of a metadata-admin form layout, and
it did not declare `dependsOn`. `widgets.tsx` held a SECOND inline description
of the same object as `WidgetProps.fieldSpec`, and that one did — because
`field-selector` (`dependsOn || reference || 'objectName'`) and
`dynamic-config` (`formData[dependsOn]` → `context.dynamicSchemas`) both read
it as their primary configuration. One value, one channel, two descriptions,
disagreeing on the key that decides what those widgets offer: so
{ field: 'fields', widget: 'field-selector', dependsOn: 'objectName' }
— the one configuration that makes that widget work — was `TS2353` for anyone
who typed their spec. It survived because in-repo specs reach the form through
`as any`, so the authoring type was never asked the question it answered wrong.
The declaration moves to a leaf module both halves import (`form-spec.ts`);
`SchemaForm.tsx` imports `./widgets.js`, so a back-edge would close a cycle.
The moved text is byte-identical apart from the new key, and `SchemaForm.tsx`
re-exports `FormFieldSpec` / `VisibilityPredicate` so no importer moves.
`string | string[]`, NOT `@object-ui/types`' wider `DependsOnInput`: both
readers index `[0]` and use it as a field name, so the object arm would arrive
where a string is required. Pinned as a refusal, not left to a later widening.
No runtime behaviour changes — the runtime already passed the key through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014zHsbJoTkTZeJQ5DLbRXrECo-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014zHsbJoTkTZeJQ5DLbRXrE
Contributor
✅ 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
|
Contributor
✅ 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
|
Uh oh!
There was an error while loading. Please reload this page.
os-sales pushed a commit
that referenced
this pull request
Aug 21, 2026
…it (#5542) objectui#5040 was not a missing key — it was that two hand-written descriptions of one contract drifted, and nothing could notice, because each was only ever checked against itself. PR #5537 converged the two app-shell descriptions into views/metadata-admin/form-spec.ts. A third survived: apps/console FormPage.tsx declared its own nine-key `interface FormFieldSpec`, under the same name, in a different package, leaving that failure mode fully available. Measured key by key before picking a route. The console's copy was a strict subset — 9 of the shared type's 26 keys, every one identical in type, none console-only — sitting in a position that describes an AUTHORED DOCUMENT: `FormSectionSpec.fields`, read straight off the /meta/view/:name payload, the same spec FormView metadata-admin renders (both files spell the same six-member `type` union and call the element type `FormFieldSpec`). The narrow, renderer-honoured shape is a different type that already exists in that file, `RenderableField`. So this was one contract described twice, and the console's description was wrong about the document: `visibleWhen`, `dependsOn`, `type`, `options`, `immutable`, the recursive `fields` and ten more legal keys were undeclared there — #5040's own symptom, "the type rejects the configuration the runtime accepts". Route: import, not rename. @object-ui/app-shell re-exports FormFieldSpec from its package root (type-only, erased at build), FormPage.tsx imports it and deletes the local declaration. Reachability is load-bearing: a type that cannot be imported is a type that gets retyped. form-spec.ts itself is untouched. FormPage.fieldSpec.test.ts is the pin. It reads the field-spec type back out of the exported buildSections signature rather than naming it, so re-inlining a local copy fails type-check even if the copy agrees on every key the day it is written. Liveness controls keep it from being a phantom check: the removed nine-key shape is pinned NOT equal to the shared type, RenderableField is pinned not equal either, and an undeclared key is still rejected. Fixes#5542 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012u2pRjcqAYtoEjgr3wwhnK
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes#5040
FormFieldSpecis the authoring surface of a metadata-admin form layout — theelement type of
FormSectionSpec.fields[], what an author actually writes. Itdid not declare
dependsOn.widgets.tsxheld a second, inline descriptionof the same object as
WidgetProps.fieldSpec, and that one did — because tworegistered widgets read it as their primary configuration:
dependsOnfield-selectordependsOn || reference || 'objectName'dynamic-configformData[dependsOn]→context.dynamicSchemas[value]One value, one channel (
MetadataFieldhands the same object down), twodescriptions — disagreeing on the one key that decides what those widgets show.
So the only configuration that makes
field-selectorwork,was
TS2353for anyone who typed their spec. Nothing was broken at runtime, andthat is exactly why it survived: in-repo specs reach the form through
as any/loose types, so the authoring type was never asked the question it answered
wrongly.
What this PR does
dependsOn?: string | string[]onFormFieldSpec.WidgetProps.fieldSpecisFormFieldSpecnow, not a structural copy of it.VisibilityPredicate, which it depends on) intoa leaf module
views/metadata-admin/form-spec.ts.SchemaForm.tsximports./widgets.js, so a back-edge fromwidgets.tsxwould close a cycle; amodule that imports nothing is importable from both sides.
SchemaForm.tsxre-exports both names, so no importer moves.
SchemaForm.widgetLabelling.test.tsx, whichnamed this gap as "filed separately rather than papered over here" — its
cases are typed
Partial<FormFieldSpec>and itsas FormFieldSpeccast isgone, so that file is now an incidental second pin.
The moved text is byte-identical apart from the one new key — verified by
reconstructing the extracted block minus the insertion and comparing it to the
pre-edit bytes.
The measurement this card actually turned on
Pointing
WidgetProps.fieldSpecatFormFieldSpecnarrows two keys:colSpan(number→1|2|3|4) andfields(any[]→Array<string | FormFieldSpec>). Whetherwidgets.tsxstill compiled under that narrowing hadnever been measured.
It compiles, with zero call-site changes. Enumerated first —
widgets.tsxreads exactly seven members off
fieldSpec(options,dependsOn,language,reference,multiple,label,field) and neithercolSpannorfieldsis among them — then confirmed by running it:No widening of
FormFieldSpecwas needed and none was made.Reverse verification — prediction written before the run
Ablation: delete only
dependsOnfromFormFieldSpec, keep everything else.No rebuild was needed or performed —
tscreads app-shellsrc/directly, andthe only thing
tsconfig.test.jsonresolves through built.d.tsis otherpackages, which the ablation does not touch. The mutation was confirmed on disk
against the exact text removed (declaration lines
1 → 0) before anything wasread.
Predicted, then observed:
widgets.tsxnow readsdependsOnoffFormFieldSpec;TS2339, ≥3 sitesTS2339atwidgets.tsx:670,:2411(×3)TS2353on PIN A/B and the three typed runtime specs;SchemaForm.widgetLabelling.test.tsx×3@ts-expect-errornegative controls stay satisfied — noTS2578TS2578anywhereTwo honest deltas from the prediction:
DynamicConfigWidget.test.tsxalso went red(5 ×
TS2353). It passesfieldSpec={{ …, dependsOn: 'driver' }}as an objectliteral, so the convergence pulled a pre-existing test under the authoring
type's checking. More diagnostics, not fewer — evidence the convergence
extended type coverage rather than just adding a key.
TS2339on the indexed access rather thanthe
TS2344I guessed. Same direction, different code.Line 1 of that table is the card in one line: those reads compiled before this
PR, because they were checked against
widgets.tsx's own copy. Line 4 is why aruntime-only verification of this card would have proved nothing.
The negative controls, and why they are not decoration
A pin that only showed
dependsOncompiling would also pass if the type hadmerely become looser. Both controls are
@ts-expect-error, which is itself anerror (
TS2578) once the line below it starts compiling — so a looseningturns the file red:
signature or
any.dependsOn: [{ field, param }]is still rejected.@object-ui/typesalready defines a canonical
DependsOnInput(packages/types/src/form.ts)that admits that object arm. It is the obvious thing to reach for and it is
wrong here:
widgets.tsx:670and:2411both index[0]and use theresult as a field name to look up in
formData, so an object would arrivewhere a string is required and break both widgets silently.
string | string[]is the shape this surface can consume. Converging the two is its own decision;
this PR pins the refusal rather than taking it quietly.
Scope
This card lands the app-shell half only. There is a third inline copy of
this contract at
apps/console/src/components/FormPage.tsx:278, which the cardand triage both frame as two descriptions converging.
apps/consoleis heldby in-flight #5504 and is out of this card's file surface, so it is untouched
here and filed as an unassigned follow-up. #5504 is not addressed by this PR.
Every edit is inside
packages/app-shell/src/views/metadata-admin/**.Verification
All evidence below was re-run against
4660062e2, confirmed to be the headthis PR is testing (
git rev-parse HEAD==git ls-remote origin <branch>,working tree clean). Every verdict is the line the gate itself printed.
On that
TS2493, since it landed inside a pin file: it was not a pin,and nothing was silenced to clear it. Line 186 is the runtime assertion
expect(fetchMock.mock.calls.map((c) => String(c[0]))). The mock was declaredvi.fn(async () => …)— zero parameters — so vitest typedmock.callsas anarray of empty tuples and
c[0]was genuinely unreachable. The fix gives themock the parameter the thing it stubs actually has
(
async (input: RequestInfo | URL) => …), so the call tuple carries theargument the assertion reads. No cast, no widened type, no pin touched — and the
assertion still discriminates:
formDatacarries a decoy under the widget'sdefault key (
objectName: 'the_fallback_object') and the assertion demands theexact URL built from the
dependsOn-named field.The 190-file
metadata-adminsuite is the complete test family for this PR'sentire file surface — every edit is inside that directory. It took two attempts:
the first hit the container's shared verify lock (
exit 99, never acquiredafter 9m00s) and was reported as not-run rather than claimed; the retry acquired
it and passed.
check:esm-specifiersis the family this diff most directly implicates — itadded two
./form-spec.jsspecifiers.check:published-distand the fullcheck:node-esm-loadload leg are not owed by this diff: both workflowstrigger only nightly, on
workflow_dispatch, or on a push touching the gate'sown script — measured from their
on:blocks, not assumed.Lint scope, stated so it is checkable.
pnpm lintisturbo run lint, i.e.each package's own
eslint .; the job for the only package this PR edits was runin full. Population read from eslint's own config rather than guessed: 898 files
(
--format jsoncount), 0 errors, and all five changed files are provably inthat population. No type-aware linting is configured (no
project/projectServiceineslint.config.js), so an app-shell edit cannot move theverdict on any file in a package this PR does not touch.
dispatch-gates.mjscould not be used to derive the family list: it resolves itsrepo root from its own location, so run from the objectui worktree it derived
against the objectstack tree and refused (exit 2) rather than answering
wrongly. The list above was derived by hand from objectui's own
.github/workflows/*.yml.Generated by Claude Code
Generated by Claude Code