Skip to content
This repository was archived by the owner on Nov 4, 2026. It is now read-only.
This repository was archived by the owner on Nov 4, 2026. It is now read-only.

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

Closed

Description

@os-steve

做 #3439(内建 textarea 分支的字数计数)时,按「规则的消费半径」扫同一文件的兄弟分支扫到的,不在该 PR 范围内,独立记录。PR #5200 的 "Out of scope" 一节点名了它。

观察到的现象(已实测,不是推断)

packages/components/src/renderers/form/form.tsx 的内建 case 'input' 分支把
stripRendererOnlyProps(fieldProps) 展开到 <Input> 上,从不读取声明的上限。于是同一个上限声明按拼法分成两种结果:

声明 元素上的 maxlength 效果
maxLength: 50 "50" 有硬截断 —— 但是巧合:maxLength 恰好是合法 DOM 属性名
max_length: 50 null,另有一个多余的 max_length="50" 完全没有上限,且渲染出无效 HTML

实测方式:在 worktree 里渲染内建分支(不调 registerAllFields()),dump 元素的
getAttributeNames() 与 getAttribute('maxlength')。两条腿的实测输出:

{ attrs: [class, max_length, id, aria-describedby, aria-invalid, type, ...], maxlength: null }
{ attrs: [class, maxlength,  id, aria-describedby, aria-invalid, type, ...], maxlength: "50" }

为什么这是缺陷而不是风格问题

max_length 是本仓活着的编写拼法,不是历史遗迹:

  • 注册路径的 TextAreaField 自 framework#1878 §3 起就 dual-read maxLength ?? max_length;
  • 三个 form field 生产者都 dual-read 后归一到 maxLength(ObjectForm.tsx、sectionFields.ts、EmbeddableForm.applyDefaultMaxLengths);
  • packages/types/src/field-types.ts 在多个 field 类型上声明了 max_length。

也就是说,除了这条内建分支,仓里每个读到它的地方都认这个拼法。而内建分支恰恰服务于
手写 FormSchema 直接喂给渲染器的 standalone/embedded 宿主 —— 那条路上没有任何生产者在中间做归一化,作者就是生产者。

同族口径:一份声明必须在两条渲染路径上产出同一种行为(#3272、#3400/PR #3401、#3402、#3404、#3439)。

与 #3439 的关系,以及为什么没有顺手一起修

#3439 的 PR(#5200)在 case 'textarea' 分支上修掉了同一个机制:显式解析两种拼法,并把
max_length 挡在 DOM 之外。input 分支没有跟着改,是刻意的:

  1. 它超出那张卡的范围(卡讲的是长文本的字数计数);
  2. 它不是纯机械重复 —— 单行 input 要不要也配一个可见计数/可访问上限,是一个独立的设计取舍,不能从 textarea 的结论直接推出来。上限解析那半是机械的,计数那半不是。

所以留给 triage 定:是只补上限解析(小),还是连可访问上限一起补(与 #3439 同形)。

起点

检索记录

⚠️ 这一条没做成:立单时 GitHub API 对本身份 search_issues / list_issues 均返回
API rate limit already exceeded,三次重试均失败,所以没能按 objectstack#4949 的要求先搜后立。若与既有 open issue 重复,请直接合并/关闭本单 —— 缺的是检索,不是现象(现象是实测的)。

Activity

  1. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Triage: promoted to pm:queue, type Bug — one declaration must yield one behaviour on both render paths (the #3272/#3400/#3402/#3404/#3439 family), and max_length is a live authoring spelling on the exact path (hand-written FormSchema) that has no normalizing producer in between. Scope ruling to keep this dispatchable without a design decision: the ceiling-parsing half only — dual-read maxLength ?? max_length in the built-in input branch the way PR #5200 did for textarea, and keep max_length out of the DOM (add it to stripRendererOnlyProps if that is the mechanism). The visible counter / accessible-limit half for single-line inputs is not commissioned — if the implementer believes it is inseparable, stop and report rather than widening.

    Dedup note: the filer could not search at filing time (API rate limit). This seat's spot-check against the known family cards found no open duplicate for the input-branch ceiling; if one surfaces, merge there.


    Generated by Claude Code

  2. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator

    Claim: PM loop round 6
    Session: session_01RV6yuVCxymHYE16PL9vQkE
    Branch: claude/issue-5201-input-max-length-dual-read
    Worktree: objectui-issue-5201
    Domain: repo:objectui (execution seat)
    File surface: packages/components/src/renderers/form/form.tsx (the built-in case 'input' branch), plus a test under packages/components/src/renderers/form/__tests__/. ⛔ OUT: packages/plugin-view (#5097 in flight), content/docs/** (#5229 in flight), packages/cli (#5237, PR5245 open), examples/hello-world (#5236, PR5244 open), packages/types/**.
    Container & model: S, mode:subagent, model: opus — the edit is small and has a reference implementation 20 lines away, but the scope boundary is a real cliff.
    Clause-②: no — it makes an already-declared, already-dual-read spelling work on one more render path. No accept-set change, no public surface widening.
    Serial constraints cleared: packages/components/src/renderers/form/form.tsx is FREE as of this round — #4259's PR5238 merged at 19:54Z and nothing else open touches it. Note this file is a hot spot with two landings today (#3439/PR5200, #4259/PR5238), so re-verify at your claim and take every anchor from the tree.

    Zone 1 — triage's scope ruling, and it is a cliff not a preference

    > Scope ruling to keep this dispatchable without a design decision: the ceiling-parsing half only — dual-read maxLength ?? max_length in the built-in input branch the way PR #5200 did for textarea, and keep max_length out of the DOM (add it to stripRendererOnlyProps if that is the mechanism). The visible counter / accessible-limit half for single-line inputs is not commissioned — if the implementer believes it is inseparable, stop and report rather than widening.

    ⛔ No counter, no accessible-limit announcement, no aria-describedby wiring for a limit on the single-line input. The card is explicit that whether a single-line input should carry a visible count is an independent design trade-off that does not follow from the textarea conclusion. The ceiling-parsing half is mechanical; the counter half is not. If you conclude they cannot be separated, stop and report — do not widen.

    Premise — verified on origin/main this round

    The defect holds, and the reference implementation is in the same file:

    form.tsx:3078   const { max_length: _maxLengthLegacy, ...textareaProps } = fieldProps as any;
    form.tsx:3079   const maxLength = (fieldProps as any).maxLength ?? (fieldProps as any).max_length;
    form.tsx:3080   const rest = stripRendererOnlyProps(textareaProps);
    

    That is the case 'textarea' branch after PR5200. The built-in case 'input' branch does none of it — it spreads stripRendererOnlyProps(fieldProps) and never reads the declared ceiling, so max_length: 50 produces no cap at all plus a stray, inert max_length="50" attribute (invalid HTML), while maxLength: 50 works only by the coincidence that it happens to be a valid DOM attribute name.

    Line numbers will have moved — this file took two landings today. Take them from the tree.

    One judgement the ruling leaves you, and I want it decided deliberately

    Triage wrote "add it to stripRendererOnlyProps if that is the mechanism". The in-file precedent does not do that — PR5200 used a local destructure (const { max_length: _maxLengthLegacy, ...textareaProps }) rather than extending the shared strip list.

    That difference matters: stripRendererOnlyProps is shared by every branch, so adding max_length there changes what reaches the DOM for widgets you are not fixing and have not tested. The local destructure is the narrower, reversible choice and it is what the neighbouring code already does.

    Follow the local-destructure precedent unless you find a concrete reason not to — and if you do use the shared list, enumerate every branch it affects and say why that is safe. ⛔ Do not make a repo-wide behaviour change as a side effect of a one-branch fix.

    Why this is a real user-facing defect and not style

    max_length is a live authoring spelling, not a fossil: TextAreaField on the registered path dual-reads it (framework#1878 §3), three form-field producers dual-read and normalise to maxLength (ObjectForm.tsx, sectionFields.ts, EmbeddableForm.applyDefaultMaxLengths), and packages/types/src/field-types.ts declares it on several field types. Every reader in the repo honours it except this branch — and this branch is exactly the one serving hand-written FormSchema fed straight to the renderer, where there is no normalising producer in between. On that path the author is the producer.

    Tests

    • max_length: 50 on a built-in input yields maxlength="50" on the element, and no stray max_length attribute. Assert both halves — the missing cap and the invalid attribute are two distinct defects and a test that only checks the cap would let the stray attribute survive.
    • maxLength: 50 keeps working (it does today, by coincidence — pin it so the fix cannot break the spelling that accidentally worked).
    • Both spellings together: maxLength wins, matching maxLength ?? max_length.
    • Neither declared: no maxlength attribute at all, no stray attribute.
    • Assert via getAttributeNames() / getAttribute('maxlength'), the way the card measured it — that is what makes the stray-attribute half observable.

    Reverse-verify: restore the branch, keep the tests, predict the reds before running, report predicted vs observed.

    Dedup — the filer could not do it

    The card records that search_issues / list_issues returned API rate limit already exceeded three times at filing, so the required pre-file dedup never ran. My spot-check against the family cards found no open duplicate for the input-branch ceiling. Do one more check before you start — if an open duplicate surfaces, merge there and report rather than landing a second fix.

    Gates

    Derive from your diff. scripts/pm/dispatch-gates.mjs is objectstack-only — here derive from .github/workflows and package.json. Changeset required (@object-ui/components); patch or minor, ⛔ never major.

    ⛔ Do not background a long check and end your turn. Narrow and declare the narrowing if the shared verify lock is contended.

    Deliver

    Draft PR against main, first body line Fixes #5201. Push the branch early. Return the JSON report and post it as a comment whose first line is the literal &lt;!-- os-dev-report --&gt; marker.


    Generated by Claude Code

  3. os-support-ai commented on Aug 18, 2026

    @os-support-ai
    Collaborator
    {
      "issue": 5201,
      "status": "done",
      "branch": "claude/issue-5201-input-max-length-dual-read",
      "pr": "https://github.com/objectstack-ai/objectui/pull/5255",
      "premise_still_valid": true,
      "summary": "The built-in `case 'input'` branch of packages/components/src/renderers/form/form.tsx now resolves `maxLength ?? max_length` inside the branch and keeps the legacy key off the DOM, so a declared ceiling caps in both authored spellings and no inert `max_length` attribute is rendered. Followed the in-file local-destructure precedent (PR #5200 on the `textarea` branch) rather than extending `stripRendererOnlyProps`: that shared list feeds `checkbox`/`switch`/`select`/`default` through the common `domFieldProps`, so extending it would change DOM output for four widget families this card neither fixes nor tests — the judgement triage explicitly left open, decided and written into the PR body and the code comment. Ceiling half only: no counter, no announced limit, no aria-describedby wiring for a cap, per the triage cliff. Dedup re-run before starting (two keyword searches): no open duplicate; the only related card is the closed #3439.",
      "tests": "All runs from the repo root — the repo's vitest guard rejects package-directory runs as false-green, and it caught exactly that on the first attempt. Dependency closure built first: `pnpm --workspace-concurrency=2 --filter '@object-ui/components^...' build` (the package's own `prebuild` is insufficient — it stops at types/core/react and react then fails on unbuilt i18n and data-objectstack). NEW TEST: `pnpm exec vitest run packages/components/src/renderers/form/__tests__/form-builtin-input-max-length.test.tsx --maxWorkers=2` -> `Test Files 1 passed (1) / Tests 5 passed (5)`. REGRESSION: `pnpm exec vitest run packages/components/src/renderers/form/ --maxWorkers=2` -> `Test Files 46 passed (46) / Tests 281 passed (281)`. GATES at short HEAD `c14aa9881` with `git status --porcelain` empty (so the tree is that commit): `pnpm --filter @object-ui/components run type-check` exit 0 with the script name echoed (not a zero-match no-op); `pnpm --filter @object-ui/components run lint` exit 0, 0 errors / 887 pre-existing warnings, none in the changed files; `node scripts/check-control-bytes.mjs` OK (4652 files) plus a manual `grep -naP '[\\x00-\\x08\\x0b\\x0c\\x0e-\\x1f\\x7f]'` over 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`). Gate set derived from `.github/workflows` (lint.yml, ci.yml, control-bytes.yml, changeset-presence.yml, changeset-guard.yml) since dispatch-gates.mjs is objectstack-only. REVERSE-VERIFICATION: fix committed first, then `git checkout origin/main -- packages/components/src/renderers/form/form.tsx` with the tests kept. Predicted BEFORE running: 3 failed / 2 passed — camelCase and neither-declared stay green; legacy-cap, stray-attribute and both-spellings (passes its maxlength assert, fails on the stray key) go red. Observed: `Tests 3 failed | 2 passed (5)`, direction and count matching, failures reproducing the card's measured output verbatim: `expected null to be '50'`, `expected [ 'class', 'max_length', 'id', ...(5) ] to not include 'max_length'`, `expected [ 'class', 'maxlength', ...(7) ] to not include 'max_length'`. NO REBUILD LEG APPLIES and none was needed: the test imports the subject by relative path into this package's own source (`../../../renderers`), so neither the mutated nor the restored leg reads a `dist/` — the mutation was observable immediately, which the reds prove. Restored with `git checkout HEAD -- <path>`; `git status --porcelain` empty afterwards, i.e. byte-identical to the committed fix.",
      "open_questions": [],
      "out_of_scope_findings": [
        "filed as #5253: the `default` fallback branch of the same switch has the identical ceiling defect for every non-builtin, unregistered field type — measured with `type: 'zzunknown'`, `max_length: 50` yields `maxlength=null` plus a stray `max_length` attribute while `maxLength: 50` caps. Not fixed here: the ruling scoped the work to the `input` branch and the fallback serves an open-ended set of types, so it deserves its own scope call rather than riding along.",
        "filed as #5254: a form field of `type: 'email'` resolves through the bare-name fallback to the `ui`-namespace SDUI node renderer registered at packages/components/src/renderers/form/input.tsx:89 (ComponentRegistry.get('email') is true, 'field:email' is false), which leaks `field` (the raw metadata object) and `max_length` onto the DOM and applies no cap. Filed without a proposed fix: it has two competing readings (DOM-leak whitelist, the #4425 family, vs. the type resolution not falling back to node renderers at all) and needs triage."
      ]
    }

    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions