Uh oh!
There was an error while loading. Please reload this page.
fix(components): read SchemaRenderer's evaluated disabled verdict, never the raw key - #7318
Merged
Merged
Conversation
…never the raw key `disabled` on a schema node is `boolean | string` — the string being a predicate. `SchemaRenderer` evaluates `disabled` / `disabledOn`, strips the raw key from the props it spreads, and forwards the answer as a real `disabled` prop. Eight widgets re-read the raw key beside that verdict, and an expression string is truthy however it evaluates. One carrier for one question (AGENTS.md #0.1); the in-tree precedent is `plugin-chatbot`'s renderer. Two runtime defects, both measured through the real renderer and the real registry before the fix: * `ui:form` destructured `disabled` off `FormSchema` and discarded the host prop as `_disabledProp`. A FALSE predicate greyed out every field, the submit button and the cancel button. * `ui:button` computed `schema.disabled || props.disabled || isLoading` and then let `toFormControlDomProps` re-declare `disabled` in the spread that follows — the key is kept even when the value is `undefined`, so the computed state was overwritten and `loading: true` rendered a spinner on a live, clickable button. The six DOM pass-throughs (`input`, `textarea`, `checkbox`, `select`, `combobox`, `collapsible`) were already correct AT RUNTIME through the host, because the forwarded prop was spread after the raw one; they now take the verdict by name so the behaviour no longer depends on spread order, and the raw read that the `disabled?: boolean` narrowings were load-bearing for is gone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NRRumy89BYdW9ogbcdHTho
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
|
This was referenced Sep 2, 2026
os-litant
marked this pull request as ready for review
September 2, 2026 06:45
Uh oh!
There was an error while loading. Please reload this page.
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
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#7238
disabledon a schema node isboolean | string— the string being a predicate.SchemaRendererevaluatesdisabled/disabledOn, strips the raw key from the props it spreads, and forwards the answer as a realdisabledprop (disabled: __disabled || undefined). Eight widgets re-read the raw key beside that verdict. An expression string is truthy however it evaluates, so the raw read is a second, wrong answer to a question the host already settled — AGENTS.md #0.1, one carrier for one question. The in-tree precedent ispackages/plugin-chatbot/src/renderer.tsx(#6169), whose shape this copies: take the verdict by name in the destructure, consume it, never re-readschema.disabled.What was measured before the fix
Every case renders through the real
SchemaRendererand the real registry, and every predicate is exercised in both polarities on the same node — that is what separates "the widget ignores the verdict" from "the widget is never disabled at all".On
67dadd602,packages/components/src/__tests__/disabled-verdict-one-carrier.test.tsxreportedTests 2 failed | 23 passed (25):The 23 passing are the discriminating controls: the TRUE-predicate polarity of every case, the literal
disabled: truepolarity, and both polarities of all six DOM pass-throughs.The two runtime defects
ui:form—form.tsxdestructureddisabled = falseoffFormSchemaand discarded the host prop asdisabled: _disabledProp. The raw value fed every field'sdisabled, the cancel button and the submit button, so a form declaringdisabled: "${data.status == 'locked'}"greyed itself out whatever the predicate said. It now consumes the verdict (hostDisabled === true) and the discard is gone.ui:button— the card predicted a predicate-string defect here. That is not what the tree does, and this PR reports the measurement rather than the prediction:isDisabledwas computed from the raw key and then overwritten, becausedisabledalso rodebuttonPropsintotoFormControlDomProps, which forwards it by name and keeps the key even when the value isundefined(pickDomPropsiteratesObject.keys). Spread afterdisabled={isDisabled}, the host's verdict won — so the predicate polarities were already correct, and the casualty was theloadingleg of the OR:loading: truewith no authored predicate rendered the spinner on a live, clickable button. Destructuringdisabledout ofpropsremoves the second writer;isDisabled = hostDisabled || isLoadingis now the only one.The six DOM pass-throughs (
input,textarea,checkbox,select,combobox,collapsible) were correct at runtime through the host — the forwarded prop was spread after the raw one — so their defect was compile-only, exactly as the card says. They now take the verdict by name, so the behaviour no longer depends on spread order.The compile half — the #7087 types applied as a scratch probe
06103d1d6(branchclaude/issue-7087-disabled-narrowing) merged in,@object-ui/typesrebuilt —packages/componentsoverrides the rootpathsmap, so it resolves@object-ui/typesthroughnode_modulestodist/index.d.ts, and an unbuilt probe would have measured the old narrowings — thenpnpm --filter @object-ui/components run type-check:error TS232267dadd602The nine, by file:
form.tsx×3, andinput/textarea/checkbox/select/combobox/collapsible×1 each.button.tsxcontributes none —propsisanythere, which is exactly why its defect was invisible to the compiler. The scratch merge was aborted before committing:packages/typesis not touched by this PR, #7087 remains open, and its own PR is reviewed on its own.Direct-usage census — nothing renders these widgets without the host in production
None of the eight files exports its component, so the only way to render one without
SchemaRendererisComponentRegistry.get(...). Every such call site was enumerated acrosspackages/,apps/andexamples/:packages/plugin-formcomposes{ type: 'form' }nodes and hands them toSchemaRendererwith no root-leveldisabled; every other consumer goes through the renderer too. So no boolean-only raw read is kept anywhere.form-renderers.test.tsx"should handle disabled state" mountedrenderComponent({ type: 'input', disabled: true })with no host in front of it. It now renders through the realSchemaRenderer, which is the only mount that can assert anything about an authoreddisabledat all.disabledon a field (fields: [{ …, disabled: true }]), which travels the untouchedfieldDisabledpath.Verification
vitest run packages/components/ packages/fields/ packages/react/f1f3fa6efTest Files 422 passed (422)/Tests 5041 passed (5041)vitest run packages/components/32b214a04Test Files 224 passed (224)/Tests 2065 passed (2065)turbo run type-check --filter=@object-ui/components(tsc --noEmit && tsc -p tsconfig.test.json— the new test file is type-checked)78be0e9b2Tasks: 9 successful, 9 total, 0error TSturbo run build --filter='@object-ui/components^...'78be0e9b2Tasks: 7 successful, 7 totalpnpm --filter @object-ui/components lint(eslint .— the package's own CI unit, not a narrowing)f1f3fa6ef✖ 944 problems (0 errors, 944 warnings)— all pre-existingcheck:control-bytes78be0e9b2✅ check-control-bytes: OK (scanned 6014 tracked text file(s); skipped 85 binary).check:vi-mock-specifiers78be0e9b2✅ check-vi-mock-specifiers: OK (…)check:vi-mock-inherit78be0e9b2✅ check-vi-mock-inherit: OK (… 118 inherit, 0 auto-mocked …)check:self-import78be0e9b2✅ No package names itself inside its own src/.check:phantom-deps78be0e9b2✅ Every in-scope import is declared by the package that publishes it.scripts/check-changeset-presence.mjs78be0e9b2✅ 10 source file(s) of 1 released package(s) changed, and this change declares 1 changeset(s)scripts/check-changeset-no-major.mjs78be0e9b2✅ No changeset declares a major bump.check:sdui-registration-pins(console built first)f1f3fa6ef✅ All 16 registration(s) a sideEffects array promises are present in the built console (517 chunks weighed; the 3 ruled control(s) are in the derived set).check:eager-closure(console built first)f1f3fa6efDeclared narrowing. The suite re-run on the final head is the one measurement that did not land: the shared verify lock was held continuously by a sibling agent's repo-wide chain. What the older greens still cover is exact rather than assumed —
git diff --stat 32b214a04 78be0e9b2 -- packages/components/srcis a single added file,page-header-action-ids.dist.spec.tsx, which matchesDIST_PIN_GLOB(packages/*/src/**/*.dist.spec.tsx) and therefore runs only in the opt-indistproject behindOBJECTUI_DIST_PINS=1— never in theunit/dom/dom-heavyprojects apackages/components/path filter drives. No production source underpackages/components/srcmoved. The merge's other content (packages/types,packages/plugin-form) is covered against this package by the green type-check on78be0e9b2. CI runs the full farm on this PR regardless.Ablation (taken after the commit, so the restore leg has a real reference).
form.tsxandbutton.tsxreverted to67dadd602, with the mutation confirmed on disk by anchor-text counts in both directions (hostDisabled→ 0 in both files;disabled = false,andschema.disabled || props.disabled→ 1 each) and a non-emptygit diff HEAD. Mutated:Tests 2 failed | 23 passed (25)— the same two cases, the 23 controls still green. Restored withgit checkout HEAD -- ABSOLUTE_PATH, proven by an emptygit diff HEADand agit hash-objectmatch against each file'sHEADblob, thenTests 25 passed (25). No rebuild leg is owed: the rootvitest.config.mtsaliases@object-ui/componentsand@object-ui/reacttopackages/*/src, so the mutated source is what runs.#7087 remains open. #7240, #6169 and #4581 are context only — none of them is closed here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01NRRumy89BYdW9ogbcdHTho
Generated by Claude Code