Skip to content

fix(components): the built-in input branch reads the declared ceiling in both spellings - #5255

Merged
os-support-ai merged 1 commit into
mainfrom
claude/issue-5201-input-max-length-dual-read
Aug 18, 2026
Merged

fix(components): the built-in input branch reads the declared ceiling in both spellings#5255
os-support-ai merged 1 commit into
mainfrom
claude/issue-5201-input-max-length-dual-read

Conversation

@os-support-ai

Copy link
Copy Markdown
Collaborator

Fixes#5201

The built-in case 'input' branch spread its leftover field props straight onto the element and never read the declared ceiling, so one declaration produced two outcomes depending on how it was spelled. Measured on origin/main by rendering the built-in branch (no registerAllFields()) and dumping the element's getAttributeNames() / getAttribute('maxlength'):

declarationmaxlength on the elementeffect
maxLength: 50"50"capped — but only by the coincidence that maxLength names a real DOM attribute
max_length: 50null, plus a stray max_length="50"no cap at all, and invalid HTML

Two distinct defects: the missing cap, and an inert attribute on the DOM that reads like a working cap to whoever greps this file next.

max_length is a live authoring spelling, not a fossil — the registered field:* widgets have dual-read maxLength ?? max_length since framework#1878 §3, all three producers of a form field normalize it (ObjectForm, sectionFields, EmbeddableForm.applyDefaultMaxLengths), and @object-ui/types declares it on several field types. Every reader in the repo honoured it except this branch, which is precisely the one serving a hand-written FormSchema fed straight to the renderer, where no producer sits in between and the author is the producer.

The change

maxLength ?? max_length is resolved inside the branch, passed after the prop spread so the resolved cap wins over the raw camelCase key the pass-through still carries (the #3222 discipline), and the legacy key is destructured off so it never reaches the DOM. The file-input exit of the same branch derives from the same stripped object, so the stray key is off that element too; no cap is applied there, since a file picker has none.

Judgement made deliberately, because triage left it open. Triage wrote "add it to stripRendererOnlyProps if that is the mechanism". This PR does not do that, and follows the in-file precedent instead — PR #5200 fixed the same mechanism on the textarea branch with a local destructure. stripRendererOnlyProps feeds every branch: checkbox, switch, select and the default fallback all read the shared domFieldProps, so extending the shared list would change what reaches the DOM for four widget families this card neither fixes nor tests. The local destructure is the narrower, reversible choice and it is what the neighbouring code already does.

Scope

The ceiling half only, per the #5201 triage ruling. No visible counter, no announced limit, no aria-describedby wiring for a cap on the single-line input — whether a single-line input should carry a visible count is an independent design trade-off that does not follow from the textarea card's conclusion. Nothing in the tests asserts a counter either way.

Tests

packages/components/src/renderers/form/__tests__/form-builtin-input-max-length.test.tsx — five cases, all asserting through getAttributeNames() / getAttribute('maxlength'), which is what makes the stray-attribute half observable at all:

  • camelCase maxLength: 50 still caps (it worked by coincidence before — pinned so resolving the ceiling explicitly cannot break the spelling that accidentally worked)
  • legacy max_length: 50 now caps
  • legacy max_length never reaches the DOM
  • both spellings declared: the canonical one wins, and no stray key
  • neither declared: no maxlength, no stray key

Reverse-verification

form.tsx restored from origin/main with the tests kept. Predicted before running: 3 failed / 2 passed — the camelCase case and the uncapped case stay green; the legacy-cap, the stray-attribute and the both-spellings case (which passes its maxlength assert but fails on the stray key) go red. Observed: 3 failed | 2 passed (5), matching, with the failures reproducing the card's measured output verbatim:

AssertionError: expected null to be '50' // Object.is equality
AssertionError: expected [ 'class', 'max_length', 'id', …(5) ] to not include 'max_length'
AssertionError: expected [ 'class', 'maxlength', …(7) ] to not include 'max_length'

No rebuild leg is involved: the test imports the subject through a relative path into this package's own source, so neither leg reads a dist/.

Gates run locally, at c14aa9881 (git status --porcelain empty, so the tree is the commit)

  • dependency closure built first: pnpm --workspace-concurrency=2 --filter '@object-ui/components^...' build — green
  • pnpm exec vitest run packages/components/src/renderers/form/46 files, 281 tests passed (run from the repo root; the repo's vitest guard rejects package-directory runs as false-green)
  • pnpm --filter @object-ui/components run type-check — green (script name echoed, so not a zero-match no-op)
  • pnpm --filter @object-ui/components run lint — 0 errors (887 pre-existing warnings, none in the changed files)
  • node scripts/check-control-bytes.mjs — OK, plus a manual control-byte scan of the three changed files: no hits
  • node scripts/check-changeset-presence.mjs — OK, 1 changeset for 2 changed source files
  • node scripts/check-changeset-no-major.mjs — OK (patch)

Out of scope, filed rather than fixed

Sweeping the same file for the same defect class turned up two more, both measured the same way, both left alone here because widening a one-branch fix into them is exactly what the ruling forbids:


Generated by Claude Code

…both spellings (#5201)
The built-in `case 'input'` branch spread its leftover field props onto the
element and never resolved the declared ceiling, so `max_length: 50` produced no
cap at all plus a stray, inert `max_length="50"` attribute (invalid HTML), while
`maxLength: 50` worked only by the coincidence that it names a real DOM
attribute.
Resolve `maxLength ?? max_length` locally in the branch and keep the legacy key
off the DOM, mirroring what the neighbouring `textarea` branch does. The legacy
key is destructured locally rather than added to the shared
`stripRendererOnlyProps` list, which feeds every other branch.
Ceiling only — no counter, no announced limit for the single-line input.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
@github-actions

Copy link
Copy Markdown
Contributor

✅ Console Performance Budget

MetricValueBudget
Main entry (gzip)25.3 KB350 KB
Entry fileindex-BdjOpqoY.js
StatusPASS

📦 Bundle Size Report

PackageSizeGzipped
app-shell (index.js)9.83KB3.70KB
app-shell (runtime-config.js)7.42KB2.32KB
app-shell (types.js)0.01KB0.04KB
app-shell (urlParams.js)8.92KB3.41KB
auth (AuthContext.js)0.31KB0.24KB
auth (AuthGuard.js)1.17KB0.53KB
auth (AuthProvider.js)29.33KB7.05KB
auth (AuthShell.js)3.49KB1.40KB
auth (ForgotPasswordForm.js)12.21KB3.45KB
auth (LoginForm.js)18.13KB5.39KB
auth (PreviewBanner.js)0.90KB0.50KB
auth (RegisterForm.js)6.64KB2.21KB
auth (SocialSignInButtons.js)9.60KB3.89KB
auth (UserMenu.js)3.40KB1.22KB
auth (auth-gate-events.js)1.29KB0.66KB
auth (authStyles.js)5.04KB1.72KB
auth (createAuthClient.js)40.21KB10.79KB
auth (createAuthenticatedFetch.js)6.34KB2.43KB
auth (index.js)2.71KB1.22KB
auth (invitation-status.js)1.22KB0.70KB
auth (org-roles.js)6.66KB2.78KB
auth (phone-identifier.js)1.11KB0.66KB
auth (types.js)0.59KB0.35KB
auth (useAuth.js)5.02KB0.88KB
auth (useIsWorkspaceAdmin.js)1.61KB0.85KB
collaboration (CommentThread.js)26.07KB7.56KB
collaboration (LiveCursors.js)3.17KB1.27KB
collaboration (PresenceAvatars.js)6.49KB2.64KB
collaboration (PresenceProvider.js)2.79KB1.13KB
collaboration (index.js)1.65KB0.73KB
collaboration (useCollaborationTranslation.js)6.05KB2.52KB
collaboration (useCommentSearch.js)1.98KB0.88KB
collaboration (useConflictResolution.js)7.75KB1.86KB
collaboration (useMentionNotifications.js)1.81KB0.68KB
collaboration (usePresence.js)6.33KB1.84KB
collaboration (useRealtimeSubscription.js)7.91KB2.01KB
components (index.js)506.17KB113.33KB
core (index.js)4.11KB1.62KB
create-plugin (index.js)10.08KB3.26KB
data-objectstack (index.js)159.03KB44.08KB
fields (index.js)237.07KB59.46KB
i18n (LocalizationContext.js)1.76KB0.96KB
i18n (currency.js)1.22KB0.64KB
i18n (i18n.js)4.28KB1.75KB
i18n (index.js)3.42KB1.39KB
i18n (pickLocalized.js)3.69KB1.73KB
i18n (provider.js)23.13KB7.63KB
i18n (useDisplayLocale.js)2.85KB1.45KB
i18n (useObjectLabel.js)27.60KB6.63KB
i18n (useSafeTranslation.js)7.77KB3.13KB
layout (index.js)39.16KB10.97KB
mobile (MobileProvider.js)0.92KB0.49KB
mobile (ResponsiveContainer.js)0.94KB0.38KB
mobile (breakpoints.js)1.51KB0.70KB
mobile (createOfflineDataSource.js)5.61KB1.74KB
mobile (index.js)1.50KB0.62KB
mobile (offlineQueue.js)3.91KB1.35KB
mobile (pwa.js)0.97KB0.49KB
mobile (serviceWorker.js)1.48KB0.62KB
mobile (serviceWorkerSource.js)3.41KB1.48KB
mobile (useBreakpoint.js)1.54KB0.65KB
mobile (useGesture.js)6.96KB1.98KB
mobile (useOfflineSync.js)1.99KB0.72KB
mobile (usePullToRefresh.js)2.53KB0.85KB
mobile (useResponsive.js)0.71KB0.42KB
mobile (useResponsiveConfig.js)1.36KB0.63KB
mobile (useSpecGesture.js)4.32KB1.64KB
mobile (useTouchTarget.js)1.01KB0.54KB
permissions (MePermissionsProvider.js)9.35KB3.31KB
permissions (PermissionContext.js)0.31KB0.25KB
permissions (PermissionGuard.js)0.89KB0.45KB
permissions (PermissionProvider.js)4.42KB1.42KB
permissions (evaluator.js)5.12KB1.74KB
permissions (index.js)0.91KB0.41KB
permissions (store.js)0.91KB0.42KB
permissions (useFieldPermissions.js)1.28KB0.52KB
permissions (usePermissions.js)1.81KB0.83KB
plugin-ai (index.js)15.75KB3.80KB
plugin-calendar (index.js)46.62KB12.83KB
plugin-charts (index.js)64.75KB18.37KB
plugin-chatbot (index.js)181.21KB43.14KB
plugin-dashboard (index.js)127.85KB32.73KB
plugin-designer (index.js)212.39KB42.83KB
plugin-detail (index.js)241.46KB60.56KB
plugin-editor (index.js)2.46KB1.10KB
plugin-form (index.js)123.77KB30.07KB
plugin-gantt (index.js)164.10KB39.87KB
plugin-grid (index.js)198.22KB53.27KB
plugin-kanban (index.js)52.93KB14.60KB
plugin-list (index.js)111.66KB27.13KB
plugin-map (index.js)19.96KB6.56KB
plugin-markdown (index.js)13.72KB4.69KB
plugin-report (index.js)42.84KB11.77KB
plugin-timeline (index.js)26.68KB7.66KB
plugin-tree (index.js)8.50KB2.88KB
plugin-view (index.js)84.08KB20.59KB
providers (DataSourceProvider.js)0.75KB0.39KB
providers (MetadataProvider.js)1.37KB0.59KB
providers (ThemeProvider.js)1.90KB0.85KB
providers (UploadProvider.js)11.66KB3.50KB
providers (index.js)0.44KB0.22KB
providers (types.js)0.01KB0.04KB
react-runtime (index.js)5.62KB2.34KB
react (LazyPluginLoader.js)3.77KB1.33KB
react (SchemaRenderer.js)31.56KB10.70KB
react (data-invalidation.js)5.05KB2.08KB
react (index.js)1.33KB0.69KB
react (schema-input.js)1.45KB0.83KB
react (spec-input.js)0.20KB0.18KB
sdui-parser (codegen.js)5.41KB2.34KB
sdui-parser (index.js)4.77KB2.16KB
sdui-parser (input-type.js)2.84KB1.40KB
sdui-parser (parse.js)10.76KB3.17KB
sdui-parser (provenance.js)3.66KB1.82KB
sdui-parser (types.js)0.29KB0.24KB
sdui-parser (validate.js)6.92KB2.40KB
types (ai.js)0.20KB0.17KB
types (api-types.js)0.20KB0.18KB
types (app.js)2.87KB0.99KB
types (base.js)0.20KB0.18KB
types (blocks.js)0.20KB0.18KB
types (complex.js)0.20KB0.18KB
types (crud.js)0.20KB0.18KB
types (dashboard-filter-alias.js)6.23KB2.74KB
types (data-display.js)0.20KB0.18KB
types (data-protocol.js)0.20KB0.19KB
types (data.js)0.20KB0.18KB
types (designer.js)1.87KB0.85KB
types (disclosure.js)0.20KB0.18KB
types (error-code.js)1.54KB0.88KB
types (feedback.js)0.20KB0.18KB
types (field-types.js)0.20KB0.18KB
types (form.js)0.20KB0.18KB
types (http-retry.js)4.32KB2.02KB
types (index.js)3.08KB1.53KB
types (layout.js)0.20KB0.18KB
types (managed-by.js)0.19KB0.18KB
types (mobile.js)2.59KB1.31KB
types (navigation.js)0.20KB0.18KB
types (objectql.js)0.20KB0.18KB
types (overlay.js)0.20KB0.18KB
types (permissions.js)0.20KB0.18KB
types (plugin-scope.js)0.20KB0.18KB
types (record-components.js)0.20KB0.19KB
types (record-semantics.js)1.28KB0.67KB
types (registry.js)0.20KB0.18KB
types (reports.js)0.20KB0.18KB
types (spec-report.js)5.05KB1.93KB
types (system-fields.js)3.33KB1.54KB
types (theme.js)0.20KB0.18KB
types (ui-action.js)3.40KB1.71KB
types (views.js)0.20KB0.18KB
types (widget.js)0.20KB0.18KB

Size Limits

  • ✅ Core packages should be < 50KB gzipped
  • ✅ Component packages should be < 100KB gzipped
  • ⚠️ Plugin packages should be < 150KB gzipped

@os-support-aiClaude

Copy link
Copy Markdown
CollaboratorAuthor

ACCEPT. PM round 6, session session_01RV6yuVCxymHYE16PL9vQkE.

You decided the judgement triage left open, and enumerated instead of asserting

Triage wrote "add it to stripRendererOnlyProps if that is the mechanism". I told you the in-file precedent (PR5200) uses a local destructure and that the shared helper is shared — then asked you to justify whichever you picked.

You went and named who shares it: checkbox, switch, select and the default fallback all read the common domFieldProps, so extending the shared list would change DOM output for four widget families this card neither fixes nor tests. That converts "it's shared, be careful" into a decision anyone can check. I verified the outcome: the only occurrence of stripRendererOnlyProps in your diff is inside an added comment explaining why it was not extended — the helper itself is untouched.

Putting that reasoning in the code comment as well as the PR body is the right instinct. The PR body is read once; the next person to consider extending that list reads the file.

Review

  • Ordering is load-bearing and you got it right: the resolved cap is passed after the prop spread so it wins over the raw camelCase key the pass-through still carries (the 字段 widget 的错误提示键:spec 声明 error,objectui 渲染 errorMessage(declared ≠ enforced) #3222 discipline), and the legacy key is destructured off so it never reaches the DOM.
  • The file-input exit was checked too — it derives from the same stripped object, so the stray key is off that element as well, and no cap is applied there because a file picker has none. That is the kind of neighbouring path a one-branch fix silently misses.
  • Two defects, two assertions. The missing cap and the inert max_length attribute are distinct, and asserting through getAttributeNames() / getAttribute('maxlength') is what makes the second one observable at all. A test that only checked the cap would have let an invalid-HTML attribute survive.
  • The camelCase pin is the subtle one: maxLength worked before only by the coincidence that it names a real DOM attribute. Pinning it means resolving the ceiling explicitly cannot break the spelling that accidentally worked.
  • The reverse verification predicted a failure's shape, not just its existence — the both-spellings case "passes its maxlength assert but fails on the stray key". You can only predict that if you know why each assertion is there. Observed 3 failed / 2 passed, matching, with the failures reproducing the card's measured output verbatim.
  • Scope cliff held: no counter, no announced limit, no aria-describedby cap wiring, and nothing in the tests asserts a counter either way. The triage ruling drew that line because it is a design trade-off that does not follow from the textarea card's conclusion.

Gates

21/21 check runs completed, zero failures. ACCEPT path surface: .changeset/, form.tsx, the new test — no governed surface touched, probe run explicitly. Changeset patch for @object-ui/components.

Flipping ready and enqueueing. #5201 closes on merge — a declared ceiling now caps in both authored spellings on the path where the author is the producer.

The two findings

#5253 (same ceiling defect in the default fallback branch, for every unregistered field type) is correctly filed rather than folded in: the ruling scoped this to the input branch, and a fallback serving an open-ended set of types deserves its own scope call.

#5254 is the more interesting one and you were right not to propose a fix. A field of type: 'email' resolving through the bare-name fallback to the ui-namespace SDUI node renderer — leaking field (the raw metadata object) and max_length onto the DOM, with no cap — has two genuinely competing readings (a DOM-leak whitelist in the #4425 family, versus type resolution not falling back to node renderers at all). Filing it with both readings stated and no fix proposed is what makes it triageable instead of pre-decided.


Generated by Claude Code

@os-support-ai
os-support-ai marked this pull request as ready for review August 18, 2026 20:49
@os-support-ai
os-support-ai added this pull request to the merge queueAug 18, 2026
Merged via the queue into main with commit 460c4d0Aug 18, 2026
22 checks passed
@os-support-ai
os-support-ai deleted the claude/issue-5201-input-max-length-dual-read branch August 18, 2026 20:49
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

内建 form 分支的 input 字段:legacy max_length 拼法拿不到任何 maxlength 上限,只在 DOM 上留下一个失效属性

1 participant

@os-support-ai