fix(fields): close the widget DOM prop leak with a whitelist toDomProps - #3313
Merged
Conversation
Field widgets forwarded leftover props to their control with a bare spread,
so anything a host handed them became a DOM attribute — including arbitrary
keys an author wrote on a field config:
<input placeholder="PH-f" zzcanary="CANARY-STR"
zzcanaryobj="[object Object]" zzcanarynum="42" …>
React 19 is completely silent about an all-lowercase unknown attribute, which
is why this went unnoticed.
Adds `toDomProps(props)` — exported from `@object-ui/fields` — and routes the
14 widgets that spread onto a host element through it: text, textarea, number,
boolean, date, datetime, time, email, phone, url, password, currency, percent
and select.
It is a whitelist, not a list of keys to drop. The largest leak source is the
open tail of author-supplied keys, not any named renderer prop: the form
renderer forwards every key it does not destructure, and SchemaRenderer spreads
the whole authored node with no strip layer at all. A blacklist of today's
renderer-only props would pass every canary above.
The forwarded set is the one `FieldWidgetComponentProps` already declares, and
a compile-time assertion ties the helper to that declaration so the two cannot
drift.
Eleven of these widgets carried `const { inputType, ...domProps } = props as
any` under a "Filter out non-DOM props" comment. `inputType` is stripped by the
form renderer before a widget sees it, so the line filtered nothing and the
comment actively misled; both are gone.
Adds a contract test that renders every registered field widget through both
hosts and fails on any attribute HTML does not define for that element. It
walks real DOM attributes rather than React warnings, asserts a validation
error genuinely rendered before scanning the error variant, and calibrates its
own judge against a clean fixture (zero findings) and a planted one (all
found).
Fixes #3291
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
…ions The first assertion only caught one direction — helper forwards a key the contract no longer declares. The reverse (contract declares a DOM key the helper never forwards) was completely silent, and the leak test structurally cannot see it: that test looks for attributes that ARRIVE, not for ones that go missing. That silent direction is "declared but not delivered" — the failure class this repo treats as first-class (objectui#3290's `aria-required` that never reached a control; objectui#3222's validation slot nobody produced). Extracts the contract's DOM pass-through block into a named `FieldWidgetDomProps`, intersected into `FieldWidgetComponentProps` (a structural no-op for every consumer), and adds the reverse assertion: `keyof FieldWidgetDomProps extends DomPassThroughKey`. Verified by breaking it: adding `role?: string` to `FieldWidgetDomProps` without adding it to `DOM_PASS_THROUGH_KEYS` now fails type-check with `toDomProps.ts(126,7): error TS2322: Type 'true' is not assignable to type 'never'`. `className` / `disabled` stay outside the named type — they are DOM-legal and forwarded, but widgets also interpret them, so they are bound in the forward direction only. The changeset now states exactly which direction each assertion guards instead of claiming the pair "cannot drift".
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #3291
范围以 #3291 上「审计结果」那条评论为准(正文已过时):14 个 widget + 2 个新文件,
form.tsx未改动,SelectField的aria-invalid缺失留给 #3306。病根:不是某个具名 prop,是「作者键的开放尾巴」
widget 用裸展开把剩余 props 转发给控件,宿主给什么就落什么到 DOM 上(下方标签的
<后统一加了空格,否则会被 GitHub 正文净化器当成 HTML 标签吃掉):zzcanaryobj="[object Object]"—— 本单命名由来的那个[object Object],换了个键仍然完整复现。它是作者写在字段配置上的普通额外键被String()到属性上。React 19 对全小写未知属性完全静默,所以它安静地活了很久。因此必须是白名单
黑名单(枚举
error/emptyHint/dataSource/dependentValues/dependsOn/options/inputType/ …)会放行上面每一个 canary,也挡不住 SDUI 路径上的label。两条路径的形状决定了这一点:SchemaRenderer把整个作者节点摊成 props,没有任何 strip 层 —— 该路径上 widget 自己的展开是唯一防线。改动
toDomProps(props),从@object-ui/fields导出(理由同已导出的withFieldCarrier:仓外 widget 作者要够得着,否则会重新长出裸展开)。text/textarea/number/boolean/date/datetime/time/email/phone/url/password/currency/percent/select。const { inputType, ...domProps } = props as any;(注释写着 "Filter out non-DOM props")连同注释一起删除 ——inputType早在宿主侧就被 strip 掉了,这行什么也没过滤,注释是主动误导。SelectField两处展开只收口了一处:Select.Root那处收口;< MultiSelectField {...props} / >是 widget 到 widget 的委托,不是 DOM 展开,narrow 掉会把它变成空组件 —— 代码里写明了这个边界。白名单与
types.ts声明的差异(需要 review 的一点)toDomProps是FieldWidgetComponentProps「DOM pass-through」段落的运行时执行体。它转发该段落声明的 7 个键 +AriaAttributes+data-*,外加 2 个:classNameclass。不转发会让toDomProps变成一个静默丢样式的助手 —— 而< Input {...toDomProps(props)} / >正是本改动教给未来每个 widget 作者的写法。disabledreadonlyOR 后使用)。不转发会静默丢交互态。即:新增一个静默失败模式,来换取白名单的字面纯粹,方向反了,所以两个都转发。若维护者认为应严格只留 7 个,我可以改为在 9 个 widget 里逐个显式传
className。反向的一个故意后果:契约没声明的 HTML 全局属性(如
role)不再转发。它此前只是搭着开放展开到达 DOM 的。若字段节点应当能写role,正确做法是先在FieldWidgetComponentProps上声明再加进白名单 —— 修契约,不是放宽消费端(AGENTS.md #0.1)。声明 = 强制(两个方向都由编译器绑住)
契约里「DOM pass-through」那一段已抽成具名类型
FieldWidgetDomProps,与FieldWidgetComponentProps交叉(对所有消费方是结构性 no-op)。toDomProps.ts里两条编译期断言:DomPassThroughKey extends keyof FieldWidgetComponentPropskeyof FieldWidgetDomProps extends DomPassThroughKey反方向正是 PM 复核点名的缺口,也是本仓视为一等缺陷的失败形态(#3290 的
aria-required声明了却没送达真实控件、#3222 的校验槽没人生产)。契约测试结构上够不着它 —— 它查的是「属性到达了」,不是「属性没到」。实测反向断言承重:往
FieldWidgetDomProps加一个role?: string而不加进DOM_PASS_THROUGH_KEYS,type-check 立刻红:(126 行正是
_everyDeclaredDomKeyIsForwarded。)还原后 11/11 type-check 通过。className/disabled只被正方向绑住 —— 它们是 DOM 合法且确实转发,但因 widget 还会解释它们而声明在 controlled-input 段,故意留在具名类型之外。changeset 里现在如实写明每条断言各保证哪个方向,不再用「so they cannot drift」这种担保不了的措辞。契约测试
packages/fields/src/__tests__/widget-dom-leak-e2e.test.tsx—— 46 个注册 widget × 5 变体(表单 plain / 表单 author-extras / 表单校验失败后 / SDUI plain / SDUI author-extras),共 233 个断言,8.4s,连跑 4 次零 flake,不需要进heavyDomTests。审计点名的四个坑都堵了:
label只在 SDUI 侧现形)。required是存在性检查(false/0是值,cloud#972),所以统一用null起始值,boolean也能真触发失败。data-*/aria-*/cmdk-*;一份 HTML 全局属性清单;核心是「属性名大小写不敏感地匹配document.createElement(tag)原型链上某个 IDL 属性」(自动覆盖readonly→readOnly/maxlength→maxLength/colspan→colSpan,无需按标签手维护);4 条别名;SVG 走自己的清单(IDL 那招在 happy-dom 下对 SVG 不成立)。判定器自证 fixture 当场赚回成本:我原以为 happy-dom 只缺 2 个 IDL(
select[size]、option[label]),干净 fixture 立刻报出另外两个真实缺口 ——textarea[wrap]和col[span],都是标准 HTML。例外表现在 5 条,每条注明理由,全部由 fixture 实测得出而非猜测。覆盖边界如实写在文件里:不驱动 popover 打开(Radix 需要 happy-dom 没有的 pointer-capture)。展开点在行内 trigger 上且总会渲染,所以展开点本身被覆盖了 —— 但只存在于展开后的内容没有。
FORM_FIELD_TYPES一致性断言让这道门自动跟随注册表:新增字段类型不进测试表 = 测试变红。破坏验证
Break A —— 把
TextField换回裸展开(const domProps = props as any):4 个用例变红,精确点名 widget、标签、属性、值、outerHTML:form/authored-extras→ 5 个:zzcanary/zzcanaryobj="[object Object]"/zzcanarynum/zzcanarycamel/reference_toform/validation-error→error="F is required"sdui/plain→label="F"sdui/authored-extras→ 6 个注意
form/plain没红、sdui/plain红了 —— 这正是审计说「只测表单路径会把 SDUI 的洞报成干净」的实测复现。还原后全绿。Break B —— 从例外表删掉
option: label:干净 fixture 立刻变红并点名< option > label="L"。证明判定器的自检是承重的,将来 happy-dom 升级导致 IDL 覆盖变化时会响亮失败,而不是静默失明。验证
packages/fields全量vitest runturbo type-checkturbo lintturbo buildchangeset:checkmajor✅changeset 标
minor(objectui 是 fixed group,绝不声明major),行为变更写在正文:作者字段配置里的未知键不再落到 DOM 属性上。未触碰content/docs/releases/。清单外发现(未在本 PR 修改)
SelectField的aria-invalid/aria-describedby/aria-required缺失 =field:select从不向辅助技术播报校验状态 —— aria-invalid / aria-describedby / aria-required 被 Radix Select.Root 静默丢弃 #3306,已裁决排在本单之后,本 PR 未顺手补。field:permission-facet-link未经withFieldCarrier注册、capability-multiselect在实际路径未注册)本 PR 同样未动。Generated by Claude Code
Generated by Claude Code