Uh oh!
There was an error while loading. Please reload this page.
fix(components): enforce the declared max_length ceiling on the form default fallback branch - #5276
Merged
os-support-ai merged 1 commit intoAug 18, 2026
Conversation
…back branch The last arm of the built-in field switch — serving a `type` that is neither a `BUILTIN_FIELD_TYPES` member nor resolvable from the registry — spread its props onto the rendered `Input` and never read the declared ceiling, so one declaration split into two outcomes by spelling. Measured on main after #5201 landed: max_length: 50 -> attrs=["class","max_length",...] maxlength=null maxLength: 50 -> attrs=["class","maxlength",...] maxlength="50" camelCase capped by coincidence (it names a real DOM attribute); the legacy `max_length` capped nothing and landed as a stray, inert attribute — invalid HTML that reads like a working cap. Two independent defects. Fixed with the same shape as the landed `input` (#5201) and `textarea` (#3439) arms: a local destructure of the legacy key plus a locally resolved `maxLength ?? max_length`, applied after the spread. The shared `stripRendererOnlyProps` table is deliberately untouched — it feeds `checkbox`, `switch` and `select` as well. Fixes#5253 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 18, 2026 23:58
Uh oh!
There was an error while loading. Please reload this page.
os-support-ai
deleted the
claude/issue-5253-form-default-branch-ceiling
branch
August 18, 2026 23:58
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#5253
The last arm of the built-in form's field switch — the
defaultfallback, serving atypethat is neither aBUILTIN_FIELD_TYPESmember nor resolvable from the registry underfield:+ type or the bare type — spread its props straight onto the renderedInputand never read the declared ceiling. It is the same defect #5201 fixed one arm earlier in the same switch, and it survived that card because #5201's triage ruling scoped the work to theinputbranch.Measured before-state
Re-measured on current
main(87d9202b1, i.e. after#5201's PR merged), rendering the built-in branch with noregisterAllFields()and a field oftype: 'zzunknown', dumpinggetAttributeNames()andgetAttribute('maxlength'):This reproduces the filer's measurement exactly, so #5201's merge did not cover this branch. One declaration split into two outcomes by spelling: camelCase
maxLengthcapped by coincidence (it happens to name a real DOM attribute), while the legacymax_lengthcapped nothing at all and landed as a stray, inertmax_length="50"attribute — invalid HTML that reads like a working cap to whoever greps this file next. Two independent defects.After the change, all four rows read
maxlength="50"/ no attribute respectively, with no stray key:Why this is a fix, not a renderer-side tolerance
max_lengthis a live authoring spelling (AGENTS.md #0.1 does not bite here): the registeredfield:*widgets have dual-readmaxLength ?? max_lengthsince framework#1878 §3, all three producers of a form field normalize it (ObjectForm,sectionFields,EmbeddableForm.applyDefaultMaxLengths), and@object-ui/typesdeclares it on several field types. This branch serves a hand-authoredFormSchemahanded straight to the renderer, where there is no normalizing producer in between and the author is the producer — so it was the one reader in the repo that dropped the declaration.Sibling implementations mirrored
The fix shape is inherited from triage, not chosen here — a local destructure, copied from the two landed siblings in the same file:
case 'input'(内建 form 分支的 input 字段:legacymax_length拼法拿不到任何 maxlength 上限,只在 DOM 上留下一个失效属性 #5201) —const { max_length: _maxLengthLegacy, ...inputProps } = domFieldProps as any;plusmaxLength ?? max_length, applied after the spread (the 字段 widget 的错误提示键:spec 声明error,objectui 渲染errorMessage(declared ≠ enforced) #3222 discipline).case 'textarea'(内建 form 分支的长文本字段完全没有字数计数,同一个 maxLength 在两条渲染路径上给出两种体验 #3439) — the same two lines againstfieldProps, thenstripRendererOnlyProps.⛔ The shared
stripRendererOnlyPropstable is deliberately untouched. It feedscheckbox,switch,selectand this fallback alike, so widening it would silently change three branches this card neither fixes nor tests — the alternative triage explicitly rejected. The only occurrence of that helper's name in this diff is inside an explanatory comment.Out of scope and unchanged: this branch still does not do the
inputarm'svalue={… ?? ''}normalization, and no{n}/{max}counter is added (the counter half remains the undecided design question #5201 left open).Tests — both halves asserted
New file
packages/components/src/renderers/form/__tests__/form-builtin-default-branch-max-length.test.tsx, copying #5201's criteria: every case readsgetAttributeNames()andgetAttribute('maxlength'), because the missing cap and the stray attribute are two distinct defects and either assertion alone passes against half the bug. Six cases: a routing guard proving thedefaultarm is really the path under test (counter-probed against a populated registry, so the twoundefinedlookups are a reading and not an empty registry), camelCase still capping, legacymax_lengthnow capping, no stray key on the DOM, canonical winning when both are declared, and an uncapped field left byte-for-byte as it was.Reverse verification
Predicted before running, with the pre-fix
form.tsxrestored and the new tests kept:max_lengthcapsexpected null to be '50'expected [ 'class', 'max_length', … ] to not include 'max_length'expected [ 'class', 'maxlength', …(6) ] to not include 'max_length'Tests 3 failed | 3 passed (6)— predicted and observed agree, including that the "canonical wins" case fails only on its stray-attribute half while its cap assertion was already green pre-fix. That is the concrete demonstration that a cap-only test would have let the invalid attribute survive.No rebuild gates this ablation: the subject is reached through a relative source import (
../../../renderers→./form), not across a packageexports→dist/boundary, so the mutation reaches the tested code directly. Restoring afterwards left the working tree byte-identical to the committed fix (git statusclean) and the suite back at 6/6 green.Verification
All run from the repo root at commit
6ce9edfcdwith a clean working tree:pnpm exec vitest run packages/components/— 163 files, 1479 tests passed, covering the siblinginput/textareaceiling suites and thecheckbox/switch/selectarms that share the untouched strip tablepnpm --filter @object-ui/components type-check— clean (tsc --noEmit && tsc -p tsconfig.test.json, so the new test file is covered; the script name echoed, confirming it was not a zero-match no-op)pnpm --filter @object-ui/components lint— 0 errors (891 pre-existing package-baseline warnings)pnpm --workspace-concurrency=2 --filter '@object-ui/components^...' build— dependency closure builds cleannode scripts/check-control-bytes.mjs— OK (4664 files), plus a directgrep -naPover the three changed filesnode scripts/check-changeset-presence.mjs— OK, 1 changeset declaredcheck:self-import,check:esm-specifiers,check:phantom-deps— all greenChangeset:
patchon@object-ui/components(nevermajor— 39 packages share onefixedgroup).Generated by Claude Code