Uh oh!
There was an error while loading. Please reload this page.
feat(lint): require an explicit type on plain button elements; fix the 114-site population (#4045) - #4450
Merged
Merged
Conversation
…e 114-site population (#4045) An HTML `button` with no `type` defaults to `type="submit"`, so it submits any form it is composed into instead of running its own handler. In an SDUI renderer that composition is a JSON metadata decision made far from the button's own file, so "not in a form today" is the dormancy, not a defence. The same defect class was patched one instance at a time three times (objectui#3344, objectstack#5236, objectstack#6952) and nothing rejected the next one at write time, because `type` is optional in React's ButtonHTMLAttributes. This adds the mechanical gate instead: - `eslint-rules/button-has-type.js` (+ RuleTester suite), wired as an ERROR over `**/*.tsx` with the population's own exclusions — `src/ui/**` (upstream Shadcn, no-touch per AGENTS.md #7) and test files. - The whole population it fires on, 114 sites in 29 files, converted to `type="button"`. No site was a genuine submit button: not one population file contains a form element. Every changed .tsx is byte-identical to its parent once the inserted attribute is removed — zero behaviour change beyond the attribute. 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
|
yinlianghui
commented
Aug 12, 2026
CollaboratorAuthor
ACCEPT — step-7 复核 by PM session
Flipping ready + arming auto-merge. The population race risk is understood: if the merge-queue rebuild reds on a newly-landed untyped button, the fix is one attribute on that site. The e2e/ lint-coverage gap (repo-root Generated by Claude Code Generated by Claude Code |
yinlianghui
marked this pull request as ready for review
August 12, 2026 09:05
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 12, 2026
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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#4045
Ends the missing-
type="button"defect class mechanically, and fixes the wholepopulation in the same PR.
An HTML
buttonelement with notypedefaults totype="submit", so it submits anyform it is composed into instead of running its own handler. In an SDUI renderer that
composition is a JSON metadata decision made far from the button's own file — so "it is
not in a form today" is the dormancy, not a defence. Per-instance patching ran three
rounds (objectui#3344, objectstack#5236, objectstack#6952) and nothing rejected the next
one at write time, because
typeis optional in React'sButtonHTMLAttributes.Per the ruling on #4045: option A, the in-repo custom rule (not the
eslint-plugin-reactdependency), population fixed in the same PR.Re-measured population — the card's table was a ~3x undercount
The card's table (39 sites, measured at
ebb579dbbwith a regex scanner) is not thepopulation. Re-measured at this branch point with the rule itself — an AST measurement,
not a text scan — the population is 114 sites in 29 files.
This is not drift. Re-running a scanner against
ebb579dbbitself finds 113 there,and the card's 39 is a strict per-file subset of it: the card's scan missed whole files,
most visibly all four plugin-designer surfaces (
PageDesigner15,ProcessDesigner15,DataModelDesigner14,ReportDesigner12 = 56 sites, none of them listed on the card).The card's conclusion is unaffected — a bigger population argues harder for option A —
but the number quoted for triage was wrong, and the class-closing gate is what makes the
count stop mattering.
Real drift since the card is small and in the expected direction:
objectDetailWidgets.tsx(4 sites) was deleted by #4365, and 2 of the 3UserFilterssites the card counted were fixed by #3948.
packages/plugin-designer/src/PageDesigner.tsxpackages/plugin-designer/src/ProcessDesigner.tsxpackages/plugin-designer/src/DataModelDesigner.tsxpackages/plugin-designer/src/ReportDesigner.tsxapps/console/src/pages/developer/ApiConsolePage.tsxpackages/app-shell/src/views/studio-design/StudioDesignSurface.tsxpackages/runner/src/LayoutRenderer.tsxpackages/plugin-map/src/ObjectMap.tsxapps/console/src/components/PerformanceDashboard.tsxapps/site/app/playground/page.tsxpackages/app-shell/src/views/MetadataInspector.tsxpackages/components/src/debug/DebugPanel.tsxpackages/plugin-ai/src/NLQueryInput.tsxpackages/plugin-designer/src/components/ConfirmDialog.tsxpackages/plugin-view/src/ViewTabBar.tsxAll 114 became
type="button". No site was a genuine submit button — not onepopulation file contains a form element at all, so the "expect ~zero" in the card's
step 4 is confirmed by measurement rather than by inspection.
Zero behaviour change beyond the attribute, proved mechanically: every changed
.tsxis byte-identical to its parent once the inserted attribute string is removed.The rule
eslint-rules/button-has-type.js+eslint-rules/button-has-type.test.js, followingno-synthetic-event-trigger.js's shape exactly (default-exported rule module,meta/messages/create, RuleTester suite in plain JS, registered ineslint-rules/index.js). Wired ineslint.config.jsas error in its own configblock, like the other scoped custom rules.
Grading notes, each decided by measurement rather than taste:
{...spread}may or may not carrytypeatruntime and no checker can tell which, so exempting it would reopen the hole for
exactly the buttons whose props come from elsewhere. Measured cost of the strict
reading: zero — the whole repo has one button with a real JSX spread attribute
(
plugin-view/ManageViewsDialog.tsx) and it already declarestype="button". Thespread case gets its own
messageId(spreadType) so the report says why the spreaddid not save it. The sibling rules gave no precedent here: none of them look at JSX.
files: ['**/*.tsx'],ignores: ['**/src/ui/**', '**/*.test.tsx', '**/__tests__/**'], mirroring thepopulation's counting rules. The
src/uiignore is load-bearing, not decorative:packages/components/src/ui/sidebar.tsx:314(SidebarRail) is an untyped button, andwithout the ignore the rule would demand an edit in a zone AGENTS.md 构建一个 **Live Playground (实时演练场)** (用于展示引擎能力) #7 forbids
touching and the Shadcn sync script overwrites. See "Known residual" below.
Buttoncomponent is a differentcontract — its own implementation owns the DOM attribute.
no-dynamic-import-in-test-hook.jsrecords for itself):
type="button"is right for nearly every site, and "nearly" isthe problem — a genuine submit button silently rewritten stops submitting its form,
invisibly to any test that does not click it.
Radix
asChildchildren — a declared deviation from the rulingThe ruling put the card's 2 Radix-trigger children out of scope
(
ApiConsolePage.tsx:274CollapsibleTrigger,AppSwitcher.tsx:49DropdownMenuTrigger),on the card's own correct finding that Radix's
Primitive.buttonalready suppliestype="button"through its Slot. Both are fixed here anyway, because the rulecannot see the difference: the alternative was leaving two permanent lint errors or
writing two
eslint-disablecomments, and a disable comment is a hole that reads assanctioned. This is runtime-neutral — Radix merges the identical value onto a child that
declares none, so the rendered DOM is unchanged either way — and it is exactly what
packages/components/src/custom/combobox.tsx:74-80already prescribes and the cardquotes approvingly: "that is an upstream implementation detail — declare the contract
locally".
Pre-fix red (#4118) — the non-vacuity proof
The rule was run against the unfixed tree first: 114 firings, all
missingType,0
spreadType. Two independent methods agree on the population — the AST rule and astandalone regex scanner — with zero disagreements in the direction that would mean
a false positive (every site the scanner flagged, the rule flagged). The rule found 2
sites the scanner missed (
ApiConsolePage.tsx:323,DataModelDesigner.tsx:751), wherean apostrophe in JSX text swallowed a region during string masking; both are genuine.
Post-fix the same run reports 0.
Verification
pnpm exec vitest run eslint-rules/button-has-type.test.js— 19 passed (10 valid /9 invalid cases).
turbo run lint --concurrency=2, repo-wide: 45/45 tasks successful, 0 errors,0
button-has-typefirings. This is the load-bearing check — the rule is live inevery package, not just the ones edited here.
pnpm exec vitest runover the 11 touched packages (repo-root invocation, pathfilters, per AGENTS.md §9): 599 files / 5779 passed, 1 skipped, 0 failed.
turbo run type-checkover the touched packages +type-check:scripts+type-check:vitest-setup: all green. As predicted, type-check does not move — thechange is attribute-only.
check-control-bytes(4147 files),check-phantom-dependencies,check-changeset-presence,check-changeset-fixed,check-changeset-no-major: green.origin/main(4 commits, incl. finding(react): bridgeListView's mapDensity carries four rowHeight spellings no spec-valid list view can hold #4352 / finding(plugin-dashboard): the #3291toDomPropswhitelist stops atpackages/fields— SDUI widgets elsewhere still close their DOM leak by hand, if at all #4425 / AiChatPage narrows useObjectChat's messages with 5 casts — route it through the exported toRuntimeMessages instead (the #4399 move, one hop up) #4437): 0 newfirings in the merged-in files, and the repo-wide lint re-run stayed 45/45 green.
Reverse verification (direction predicted before running)
typefrom one fixed site reds the rule naming exactly thatsite. Observed:
drawer.tsx:38:56 object-ui/button-has-type missingType, exactlyone firing. Restored.
eslint.config.jsmakes lint green again — i.e. the wiring is load-bearing, notjust the rule file. Observed: 0
button-has-typemessages. Restored.Known residual
packages/components/src/ui/sidebar.tsx:314(SidebarRail) is still an untyped button.It sits in the upstream Shadcn zone that the sync script overwrites and AGENTS.md #7
forbids editing, so it is out of the rule's reach by the card's own counting rules. It
is upstream's to fix; filed separately as an observation rather than patched here.
What is NOT absorbed
PR #3948's scan-style assertion in
plugin-list/src/__tests__/UserFilters.test.tsxandthe family prototype
components/src/__tests__/combobox-trigger-type.test.tsxstay.They assert the rendered DOM — including buttons produced by dependencies, which a
source-static rule cannot see — and that clicking does not submit an enclosing form,
which lint cannot assert at all. Complementary, not superseded.
objectstack#7074 thread compliance
The source thread was read before acting, per the card's mandate. It carries the
finding-triage promotion (
findingtopm:queue, 2026-08-10, spot-checked live) andthe migration note closing it as moved, not rejected under objectstack#7167 — no hold
or restart condition applies. Its substantive ruling, the Radix-mechanism correction, is
honoured above and its reasoning is recorded in the rule's own header so the next reader
does not re-derive it.
Generated by Claude Code