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
38 changes: 38 additions & 0 deletions .changeset/form-default-branch-max-length-ceiling.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
@@ -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:<type>` or `<type>`.
* 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(
<Form schema={{ type: 'form', showSubmit: false, showCancel: false, fields }} />,
);
}

const fallbackField = (extra: Record<string, unknown> = {}) => ({
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');
});
});
49 changes: 46 additions & 3 deletions packages/components/src/renderers/form/form.tsx
Original file line numberDiff line numberDiff line change
Expand Up@@ -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 (
<Input
type={inputType || 'text'}
placeholder={placeholder}
className={cn(readonlyInputClass)}
{...domFieldProps}
{...fallbackProps}
// After the spread, so the resolved cap wins over the raw camelCase
// key `fallbackProps` still carries (the #3222 discipline).
// `undefined` when neither spelling was declared, which renders no
// attribute — an uncapped field is left exactly as it was.
maxLength={maxLength}
onClick={(e) => {
openNativePickerOnClick(inputType)?.(e);
domFieldProps.onClick?.(e);
fallbackProps.onClick?.(e);
}}
readOnly={readonly}
/>
);
}
}
}
Loading