fix(plugin-chatbot): widgets pass only whitelisted DOM props to the host element (#4431) - #4485
Merged
Merged
Conversation
…ost element Lift the objectui#3291 whitelist MECHANISM to @object-ui/core (utils/dom-props.ts) and convert both spreading chat registrations to consume-or-whitelist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
…dger The two-way ratchet: with the rows still present the sweep goes red on `expected [] to deeply equal [ 'ariadescribedby', …(13) ]` — a row cannot outlive the defect it records. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. #4432 dispatches now, gated on this PR's landing; #4453 unlocks behind it (with the #4484 note that Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 19:37
This was referenced Aug 12, 2026
yinlianghui
pushed a commit
that referenced
this pull request
Aug 13, 2026
`generateTimeScaleHeaders` is reachable from the package entry (`index.tsx` does `export * from './renderer'`) and its published declaration grew an optional trailing `locale?: string`. Entry-reachable additive API growth is minor, not patch — patch is for changes with no API-surface movement at all. `dist/index.d.ts` being byte-identical does not argue for patch: the entry re-exports by reference, so the resolved public surface moved even though the entry file's bytes did not. The contrapositive of #4496, which was graded patch precisely because its .d.ts additions were NOT re-exported from the entry; #4403 / #4177 / #4485 / #4495-regrade are the line this follows. Additive and back-compatible is what minor means — no consumer breaks, existing three-argument callers keep compiling and keep producing byte-identical output. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
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.
Closes #4431
Migration step 1 of the #4425 phase-2 ruling (comment): option 1 — promote the whitelist to the SDUI widget contract. This PR carries the shared infrastructure the later per-package cards (#4432, #4453) consume.
The defect
Both registrations in
packages/plugin-chatbot/src/renderer.tsxdestructuredschemaandclassNameand forwarded everything else intoChatbot/ChatbotEnhanced, whose props extendHTMLAttributes< HTMLDivElement >and spread the leftovers onto their root div.SchemaRendererhands a registered component the authored node's keys, the contents of itspropscontainer, the resolved ARIA, the evaluateddisabledverdict and the host's trailing props — so all of it became attributes. Measured through the real SDUI path with an adapter attached: 14 non-DOM attributes on each widget.Red-first, both directions
Defect side — pre-fix, with the ledger rows temporarily removed so the leak is visible:
(the tag name is unbracketed above only because the stored body strips a bracket followed by a letter)
Ratchet side — post-fix, with both ledger rows still in place. This is the direction that makes the row expire with the defect:
Both rows deleted in this PR →
Test Files 1 passed (1) · Tests 39 passed (39).The helper home, measured (#4409 method)
plugin-chatbotdeclares@object-ui/coreand does not declare@object-ui/fields, so consuming the #3291 whitelist frompackages/fieldswould have meant a new cross-plugin dependency. The mechanism therefore lifts to@object-ui/core(src/utils/dom-props.ts):pickDomProps(filter by a declared key list plus thearia-*/data-*open families) andtoDomProps(that mechanism with the SDUI contract's key set applied).The key list did not move, because measurement showed it is not one list:
id,className,tabIndex,autoFocus,onClick,onBlur,onFocusname,disablednameon a container div is one of the 14 leaked attributes. Two field widgets already hand-stripnamewhen spreading onto something that is not a control (ObjectRefField's trigger,FileField's dropzone).roleSchemaRenderer.resolveAriaPropsresolves it for every node from the spec'sAriaPropsSchema; the field contract deliberately does not declare it. Withholding it on the SDUI path would be declared-but-not-delivered.So
packages/fieldsis not a bare re-export: it keeps its own declared list and its two compile-time assertions, and executes them through core's mechanism. Its behaviour is unchanged and its exportedDomPropsis the same structural type. A third assertion now binds the two lists, withrolenamed as the single deliberate exception — reverse-verified by adding a key to the shared SDUI set and watching fields go red:…green again once restored.
The two triage-flagged items, pinned by name
packages/plugin-chatbot/src/__tests__/renderer.domProps.test.tsxrenders both widgets throughSchemaRendererProviderwith an adapter attached — a schema-only fixture cannot seedataSource, which is how PR #4428 shipped a six-key first pass — and asserts the host element's attribute set exactly, so a key that stops being delivered is as red as a key that leaks:datasourcegone, plus a generic assertion that no attribute value contains[object Object];arialabel/ariadescribedbygone, while the resolvedaria-label/aria-describedbyremain with their values.chatbot-floatingis untouched (measured clean — portal-mounted root).The
disabledconsequence, stated exactlyRemoving the raw spread means the injected
disabledhas to be consumed by name, and that key was never inert:SchemaRendererpassesdisabled: _disabled || undefinedlast, so the old trailing spread overrode whatever the registration had just computed. Effective behaviour through the SDUI host was therefore_disabledalone — the evaluated verdict on the node'sdisabled/disabledOn.Both registrations now consume that verdict (
hostDisabled) instead of re-reading the rawschema.disabledbeside it: one carrier for one question (AGENTS.md #0.1), and the raw value may be an expression string, which is truthy however it evaluates. Two edges change, neither silently:chatbotkeeps its own|| isLoading, which the old spread nullified. The legacy composer is now disabled while a reply is pending — what the source always said it did.chatbot-enhancedis unaffected: it passesisLoadingon its own prop.SchemaRenderer, as this package's own seam test does) gets no injected verdict, so an authoreddisabled: trueno longer disables there. On the SDUI path — the only path the engine uses — the verdict covers it.Verification
pnpm exec vitest run packages/core/ packages/fields/ packages/plugin-chatbot/—Test Files 187 passed (187) · Tests 3372 passed (3372), including the fields DOM-leak e2e gate.pnpm exec vitest run packages/app-shell/src/__tests__/widget-dom-leak-sweep.test.tsx— 39 passed.type-check(bothtscprojects) for core, fields, plugin-chatbot, app-shell — all Done.lintfor the same four — 0 errors.check-changeset-presence/check-changeset-no-major/check-control-bytes/check-phantom-dependencies/check-lint-coverage/check-type-check-coverage— all green.Changesets by emitted-
.d.tsmeasurement:@object-ui/coreminor (new module + barrel line),@object-ui/plugin-chatbotpatch (no.d.tsdelta — behaviour only),@object-ui/fieldspatch (DomPropsbecomes an alias to core's, structurally identical; no export added or removed).Generated by Claude Code