diff --git a/.changeset/builtin-input-max-length-dual-read-5201.md b/.changeset/builtin-input-max-length-dual-read-5201.md new file mode 100644 index 000000000..9e9f21113 --- /dev/null +++ b/.changeset/builtin-input-max-length-dual-read-5201.md @@ -0,0 +1,40 @@ +--- +'@object-ui/components': patch +--- + +The built-in form `input` branch now honours a declared ceiling in both authored spellings. + +The branch spread its leftover field props straight onto the element and never +read the declared ceiling, so one declaration produced two different outcomes. +Measured on `origin/main`, rendering the built-in branch (no `registerAllFields()`) +and dumping the element's `getAttributeNames()` / `getAttribute('maxlength')`: + +| declaration | `maxlength` on the element | effect | +|---|---|---| +| `maxLength: 50` | `"50"` | capped — but only by the coincidence that `maxLength` names a real DOM attribute | +| `max_length: 50` | `null`, 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 the 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 to normalize it and +the author is the producer. This is the same mechanism objectui#3439 resolved +for the built-in `textarea` branch. + +The legacy key is destructured off locally rather than added to the shared +`stripRendererOnlyProps` list: that helper feeds every branch +(`checkbox`/`switch`/`select`/`default` all share `domFieldProps`), so extending +it would change what reaches the DOM for widgets this change neither fixes nor +tests. The neighbouring `textarea` branch strips it the same local way. + +Scope, stated because the sibling card resolved more than this one: the ceiling +only. Whether a single-line input should also carry the visible `{n}/{max}` +counter and the announced limit that the `textarea` branch grew in +objectui#3439 is an independent design trade-off that does not follow from that +card's conclusion, and is deliberately left undecided here. diff --git a/packages/components/src/renderers/form/__tests__/form-builtin-input-max-length.test.tsx b/packages/components/src/renderers/form/__tests__/form-builtin-input-max-length.test.tsx new file mode 100644 index 000000000..49daa7490 --- /dev/null +++ b/packages/components/src/renderers/form/__tests__/form-builtin-input-max-length.test.tsx @@ -0,0 +1,111 @@ +/** + * 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 (unregistered) `input` branch honours a declared ceiling in + * BOTH authored spellings — objectui#5201. + * + * ## What was measured on `origin/main` + * + * 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 | + * + * `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 is the one 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()` / `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. + * Reading the attribute NAMES is what makes the stray half observable at all — + * it is how the card measured it. + * + * ## Scope + * + * The ceiling only. Whether a single-line input should also carry the visible + * `{n}/{max}` counter and the announced limit that the built-in `textarea` + * branch grew in objectui#3439 is an independent design trade-off that does + * not follow from that card's conclusion; the #5201 triage ruling explicitly + * left it undecided, so nothing here asserts a counter either way. + */ + +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'; + +/** 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 textField = (extra: Record