From 6ce9edfcd694912a0708285f4570c5d9071f68d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 23:25:10 +0000 Subject: [PATCH] fix(components): enforce declared max_length on the form default fallback branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE --- .../form-default-branch-max-length-ceiling.md | 38 +++++ ...builtin-default-branch-max-length.test.tsx | 143 ++++++++++++++++++ .../components/src/renderers/form/form.tsx | 49 +++++- 3 files changed, 227 insertions(+), 3 deletions(-) create mode 100644 .changeset/form-default-branch-max-length-ceiling.md create mode 100644 packages/components/src/renderers/form/__tests__/form-builtin-default-branch-max-length.test.tsx diff --git a/.changeset/form-default-branch-max-length-ceiling.md b/.changeset/form-default-branch-max-length-ceiling.md new file mode 100644 index 000000000..08cf9c9f7 --- /dev/null +++ b/.changeset/form-default-branch-max-length-ceiling.md @@ -0,0 +1,38 @@ +--- +'@object-ui/components': patch +--- + +The built-in form's `default` fallback branch now enforces a declared `max_length` ceiling. + +The last arm of the field switch — the one serving a `type` that is neither a +built-in field type nor resolvable from the registry — spread its props straight +onto the rendered `Input` and never read the declared ceiling. One declaration +therefore split into two outcomes depending on how it was spelled. Measured on +`main` after objectui#5201 landed: + + max_length: 50 -> attrs=["class","max_length",...] maxlength=null + maxLength: 50 -> attrs=["class","maxlength",...] maxlength="50" + +The camelCase spelling capped by coincidence — it happens to name a real DOM +attribute. The legacy `max_length` capped nothing at all and landed as a stray, +inert `max_length="50"` attribute: invalid HTML that reads like a working cap to +the next reader. Two independent defects, both now fixed. + +`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. This branch serves a hand-authored `FormSchema` handed +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. + +Same defect and same fix shape as objectui#5201 (the `input` arm) and +objectui#3439 (the `textarea` arm): the ceiling is resolved locally inside the +branch, and the legacy key is destructured off locally. The shared +renderer-only strip table is deliberately unchanged — it feeds the `checkbox`, +`switch` and `select` arms too, and widening it would alter branches this change +does not test. + +A field that declares no ceiling in either spelling renders no `maxlength` +attribute, exactly as before. diff --git a/packages/components/src/renderers/form/__tests__/form-builtin-default-branch-max-length.test.tsx b/packages/components/src/renderers/form/__tests__/form-builtin-default-branch-max-length.test.tsx new file mode 100644 index 000000000..cc796f9c5 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-builtin-default-branch-max-length.test.tsx @@ -0,0 +1,143 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The built-in `default` FALLBACK branch honours a declared ceiling in BOTH + * authored spellings — objectui#5253. + * + * ## Which branch this is + * + * The last arm of the field switch, serving a `type` that is neither a + * `BUILTIN_FIELD_TYPES` member (`input`/`textarea`/`checkbox`/`switch`/ + * `select`) nor resolvable from the registry under `field:` or ``. + * The tests below pin that routing explicitly rather than assuming it: if a + * future card registers the probe type, the guard fails loudly instead of + * quietly measuring a different render path. + * + * ## What was measured on `origin/main` (87d9202b1, i.e. AFTER #5201 landed) + * + * The branch spread `stripRendererOnlyProps(fieldProps)` onto the element and + * never read the declared ceiling, so one declaration split into two outcomes + * depending on how it was spelled: + * + * | declaration | `maxlength` on the element | effect | + * |---|---|---| + * | `maxLength: 50` | `"50"` | capped — but by COINCIDENCE: `maxLength` happens to name a real DOM attribute | + * | `max_length: 50` | `null`, plus a stray `max_length="50"` | NO cap at all, and invalid HTML | + * + * Verbatim probe output from that tree: + * + * max_length: 50 → attrs=["class","max_length","id","aria-describedby","aria-invalid","type","name"] maxlength=null + * maxLength: 50 → attrs=["class","maxlength","id","aria-describedby","aria-invalid","type","name"] maxlength="50" + * + * This is the same defect #5201 fixed one arm earlier in the same switch, and + * the fix here is the same shape: a LOCAL destructure, never a widening of + * `stripRendererOnlyProps` (that helper feeds `checkbox`, `switch`, `select` + * and this fallback alike, so widening it would change three branches this + * card does not test). + * + * `max_length` is a live authoring spelling, not a fossil: the registered + * widgets dual-read `maxLength ?? max_length` (framework#1878 §3), all three + * producers of a form field normalize it (`ObjectForm`, `sectionFields`, + * `EmbeddableForm.applyDefaultMaxLengths`) and `packages/types` declares it on + * several field types. This branch, like the `input` one, is a path with no + * producer in between — a hand-written `FormSchema` fed straight to the + * renderer — so on it the author IS the producer and nothing normalizes the + * spelling. + * + * ## Why the assertions read `getAttributeNames()` AND `getAttribute()` + * + * The missing cap and the stray attribute are two DISTINCT defects, and a test + * that only checked the cap would let the invalid attribute survive the fix — + * demonstrably so: with BOTH spellings declared, `getAttribute('maxlength')` + * was already `"50"` before the fix while the stray `max_length` was still on + * the element. Reading the attribute NAMES is what makes that half observable, + * and it is how the card measured it. + */ + +import React from 'react'; +import { describe, it, expect, afterEach } from 'vitest'; +import { render, cleanup } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { ComponentRegistry } from '@object-ui/core'; +// Module scope, not `beforeAll` — the cold transform must not be billed to +// `hookTimeout`. See object-ui/no-dynamic-import-in-test-hook (objectui#3010). +import '../../../renderers'; + +/** + * A `type` no builtin arm claims and nothing registers, so the field switch + * falls through to `default`. Same probe type the card measured with. + */ +const FALLBACK_TYPE = 'zzunknown'; + +/** Render the built-in branch: no `registerAllFields()`, so nothing resolves from the registry. */ +function renderForm(fields: any[]) { + const Form = ComponentRegistry.get('form')!; + return render( +
, + ); +} + +const fallbackField = (extra: Record = {}) => ({ + name: 'title', + label: 'Title', + type: FALLBACK_TYPE, + ...extra, +}); + +const input = () => document.querySelector('input') as HTMLInputElement; + +afterEach(cleanup); + +describe('built-in default fallback — the declared ceiling (objectui#5253)', () => { + it('is really reached through the DEFAULT arm — nothing resolves this type', () => { + // The routing guard for everything below. `ComponentRegistry.get('form')` + // in `renderForm` is the counter-probe: the registry IS populated by the + // `../../../renderers` import, so these two `undefined`s are a reading, + // not an empty registry. + expect(ComponentRegistry.get('form')).toBeTruthy(); + expect(ComponentRegistry.get(`field:${FALLBACK_TYPE}`)).toBeUndefined(); + expect(ComponentRegistry.get(FALLBACK_TYPE)).toBeUndefined(); + }); + + it('applies a camelCase maxLength as the native attribute', () => { + // This spelling worked before the fix, by the coincidence that it names a + // real DOM attribute. Pinned so resolving the ceiling explicitly cannot + // break the spelling that accidentally worked. + renderForm([fallbackField({ maxLength: 50 })]); + expect(input().getAttribute('maxlength')).toBe('50'); + }); + + it('applies the LEGACY max_length spelling too — it capped nothing before', () => { + renderForm([fallbackField({ max_length: 50 })]); + expect(input().getAttribute('maxlength')).toBe('50'); + }); + + it('never leaks max_length onto the DOM as a stray attribute', () => { + renderForm([fallbackField({ max_length: 50 })]); + // Not a DOM attribute in any spelling. Left in the pass-through it renders + // invalid HTML that looks like a working cap to the next reader — the + // second, independent half of this defect. + expect(input().getAttributeNames()).not.toContain('max_length'); + }); + + it('lets the canonical spelling win when both are declared', () => { + // `maxLength ?? max_length` — the resolution order every other reader in + // the repo already uses. Note the two halves fail independently here: the + // cap assertion passed before the fix, the stray one did not. + renderForm([fallbackField({ maxLength: 50, max_length: 80 })]); + expect(input().getAttribute('maxlength')).toBe('50'); + expect(input().getAttributeNames()).not.toContain('max_length'); + }); + + it('leaves an uncapped field exactly as it was — no attribute either way', () => { + renderForm([fallbackField()]); + expect(input().getAttributeNames()).not.toContain('maxlength'); + expect(input().getAttributeNames()).not.toContain('max_length'); + }); +}); diff --git a/packages/components/src/renderers/form/form.tsx b/packages/components/src/renderers/form/form.tsx index 147092ec6..e1ef9bc85 100644 --- a/packages/components/src/renderers/form/form.tsx +++ b/packages/components/src/renderers/form/form.tsx @@ -3215,19 +3215,62 @@ function renderFieldComponent(type: string, props: RenderFieldProps) { ); } - default: + default: { + // The declared ceiling is resolved HERE, in BOTH authored spellings, + // rather than left to ride the pass-through onto the DOM + // (objectui#5253). Identical mechanism, identical reasons and identical + // shape to the `input` branch above (objectui#5201) and the `textarea` + // branch (objectui#3439) — this fallback was simply out of #5201's + // scoped surface, so it kept the defect after that card landed. + // + // Measured on `origin/main` at 87d9202b1, with a `type` that is neither + // a `BUILTIN_FIELD_TYPES` member nor a registered component (so this + // branch renders it), the pass-through answered the two spellings + // differently: + // + // max_length: 50 → attrs=[…,"max_length",…] maxlength=null + // maxLength: 50 → attrs=[…,"maxlength",…] maxlength="50" + // + // i.e. camelCase capped by COINCIDENCE (it names a real DOM attribute), + // while the legacy `max_length` capped NOTHING and landed as a stray, + // inert `max_length="50"` attribute — invalid HTML that reads like a + // working cap to the next reader. Two independent defects. + // + // `maxLength ?? max_length` is not a tolerance invented at a consumer + // (AGENTS.md #0.1): the registered `field:*` widgets have dual-read it + // since framework#1878 §3, all three producers of a form field do + // (`ObjectForm`, `sectionFields`, `EmbeddableForm.applyDefaultMaxLengths`) + // and `packages/types`' field types declare `max_length`. This branch — + // like the `input` one — serves a hand-authored `FormSchema` handed + // straight to the renderer, where there is no normalizing producer in + // between and the author IS the producer. + // + // The legacy key is destructured off LOCALLY — NOT added to + // `stripRendererOnlyProps` — because that helper feeds EVERY branch + // (`checkbox`, `switch`, `select` and this fallback all share + // `domFieldProps`), so extending it would change what reaches the DOM + // for widgets this card neither fixes nor tests. Both landed siblings + // strip it the same local way. + const { max_length: _maxLengthLegacy, ...fallbackProps } = domFieldProps as any; + const maxLength = (fieldProps as any).maxLength ?? (fieldProps as any).max_length; return ( { openNativePickerOnClick(inputType)?.(e); - domFieldProps.onClick?.(e); + fallbackProps.onClick?.(e); }} readOnly={readonly} /> ); + } } }