Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 40 additions & 0 deletions .changeset/builtin-input-max-length-dual-read-5201.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
@@ -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(
<Form schema={{ type: 'form', showSubmit: false, showCancel: false, fields }} />,
);
}

const textField = (extra: Record<string, unknown> = {}) => ({
name: 'title',
label: 'Title',
type: 'input',
...extra,
});

const input = () => document.querySelector('input') as HTMLInputElement;

afterEach(cleanup);

describe('built-in input — the declared ceiling (objectui#5201)', () => {
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([textField({ maxLength: 50 })]);
expect(input().getAttribute('maxlength')).toBe('50');
});

it('applies the LEGACY max_length spelling too — it capped nothing before', () => {
renderForm([textField({ max_length: 50 })]);
expect(input().getAttribute('maxlength')).toBe('50');
});

it('never leaks max_length onto the DOM as a stray attribute', () => {
renderForm([textField({ max_length: 50 })]);
// Not a DOM attribute in any spelling. Left in the pass-through it renders
// invalid HTML that reads 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.
renderForm([textField({ 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([textField()]);
expect(input().getAttributeNames()).not.toContain('maxlength');
expect(input().getAttributeNames()).not.toContain('max_length');
});
});
60 changes: 53 additions & 7 deletions packages/components/src/renderers/form/form.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -3005,26 +3005,72 @@ function renderFieldComponent(type: string, props: RenderFieldProps) {
const readonlyInputClass = readonly && 'bg-muted/40 cursor-default focus-visible:ring-0';

switch (type) {
case 'input':
case 'input': {
// The declared ceiling is resolved HERE, in BOTH authored spellings,
// rather than left to ride the pass-through onto the DOM
// (objectui#5201). This is the same mechanism the `textarea` branch
// below resolves for the same reason (objectui#3439) — this branch was
// deliberately left out of that card because the COUNTER half is a
// design question for a single-line input; the ceiling half is not.
//
// Measured on `origin/main`, the pass-through answered the two spellings
// differently: a camelCase `maxLength` happened to work because it names
// a real DOM attribute, so the element got `maxlength="50"`; the legacy
// `max_length` reached the same element as a STRAY, inert
// `max_length="50"` attribute and the field had no cap at all — no
// truncation, and invalid HTML that reads like a working cap to whoever
// greps this file next.
//
// `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`. Every reader in
// the repo honoured it except this branch — which is precisely the one
// serving 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 the `default` fallback all share
// `domFieldProps`), so extending it would change what reaches the DOM
// for widgets this card neither fixes nor tests. The `textarea` branch
// strips it the same local way.
//
// Scope: the CEILING only. Whether a single-line input should also carry
// a visible `{n}/{max}` counter and an announced limit the way the
// `textarea` branch does is an independent design trade-off that does
// NOT follow from #3439's conclusion, and is deliberately not decided
// here (the objectui#5201 triage ruling).
const { max_length: _maxLengthLegacy, ...inputProps } = domFieldProps as any;
const maxLength = (fieldProps as any).maxLength ?? (fieldProps as any).max_length;
if (inputType === 'file') {
// File inputs cannot be controlled with value prop
const { value, ...fileProps } = domFieldProps;
return <Input type="file" placeholder={placeholder} className="min-h-[44px] sm:min-h-0" {...fileProps} />;
// File inputs cannot be controlled with value prop. No cap applies to a
// file picker, but the stray legacy key must not reach it either — it
// is off `inputProps` already.
const { value, ...fileProps } = inputProps;
return <Input type="file" placeholder={placeholder} className="min-h-[44px] sm:min-h-0" {...fileProps} />;
}
return (
<Input
type={inputType || 'text'}
placeholder={placeholder}
className={cn('min-h-[44px] sm:min-h-0', readonlyInputClass)}
{...domFieldProps}
{...inputProps}
// After the spread, so the resolved cap wins over the raw camelCase
// key `inputProps` still carries (the #3222 discipline). `undefined`
// when neither spelling was declared, which renders no attribute.
maxLength={maxLength}
onClick={(e) => {
openNativePickerOnClick(inputType)?.(e);
domFieldProps.onClick?.(e);
inputProps.onClick?.(e);
}}
readOnly={readonly}
value={domFieldProps.value ?? ''}
value={inputProps.value ?? ''}
/>
);
}

case 'textarea': {
// `mobile_fullscreen` is the flag's ONE spelling (objectui#3303). This
Expand Down
Loading