fix(noUnknownAttribute): add closedby as a valid attribute for <dialog>#9142
fix(noUnknownAttribute): add closedby as a valid attribute for <dialog>#9142ematipico merged 1 commit intobiomejs:mainfrom
closedby as a valid attribute for <dialog>#9142Conversation
🦋 Changeset detectedLatest commit: ee475a8 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull request overview
Fixes Biome’s lint/nursery/noUnknownAttribute false positive by recognizing the closedby attribute as valid on <dialog> elements (per issue #9141).
Changes:
- Added
closedbyto the tag-restricted attribute allowlist fordialog. - Updated the
noUnknownAttributeJSX spec fixtures and snapshots to coverclosedbyas valid on<dialog>and invalid on other tags. - Added a changeset entry to publish the fix as a patch.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/biome_js_analyze/src/lint/nursery/no_unknown_attribute.rs | Allows closedby only on <dialog> via ATTRIBUTE_TAGS_MAP. |
| crates/biome_js_analyze/tests/specs/nursery/noUnknownAttribute/valid.jsx | Adds a valid <dialog closedby="any" ... /> usage to the fixture. |
| crates/biome_js_analyze/tests/specs/nursery/noUnknownAttribute/valid.jsx.snap | Updates snapshot output for the valid fixture. |
| crates/biome_js_analyze/tests/specs/nursery/noUnknownAttribute/invalid.jsx | Adds an invalid <div closedby="any" /> usage to ensure restriction is enforced. |
| crates/biome_js_analyze/tests/specs/nursery/noUnknownAttribute/invalid.jsx.snap | Updates snapshot output for the invalid fixture and shifted diagnostics. |
| .changeset/fix-closedby-dialog-attribute.md | Patch-level release note for the fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5bf4d17 to
ee475a8
Compare
WalkthroughThis change adds the HTML attribute Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/nursery/no_unknown_attribute.rs (1)
117-117: LGTM — directly fixes the false positive forclosedbyon<dialog>.One thing worth noting: the HTML attribute is
closedby(lowercase), while the JavaScript property isclosedBy(camelCase). The existing dialog-specific entries inATTRIBUTE_TAGS_MAPuse the React/JS property form (returnValue,onClose,onCancel), soclosedBywould be the consistent key here. As-is, users writing<dialog closedBy="any" />(camelCase, following React convention) would still get anUnknownPropdiagnostic.Consider whether this should be a two-part fix:
✨ Suggested follow-up for complete coverage
- ("closedby", &["dialog"]), + ("closedBy", &["dialog"]),And in
DOM_ATTRIBUTE_NAMES(to steer users from the HTML form to the React prop form):+ ("closedby", "closedBy"),That said, since the reported issue is specifically about
closedby(lowercase), this is a nice-to-have and can be addressed in a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/biome_js_analyze/src/lint/nursery/no_unknown_attribute.rs` at line 117, ATTRIBUTE_TAGS_MAP currently uses the HTML lowercase "closedby" key which causes React-style camelCase props like closedBy to still trigger UnknownProp; update the dialog-specific entry in ATTRIBUTE_TAGS_MAP to use "closedBy" (camelCase) instead of "closedby", and add a corresponding entry in DOM_ATTRIBUTE_NAMES mapping "closedby" -> "closedBy" so users are guided from the HTML attribute form to the React prop form and both forms are handled consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/nursery/no_unknown_attribute.rs`:
- Line 117: ATTRIBUTE_TAGS_MAP currently uses the HTML lowercase "closedby" key
which causes React-style camelCase props like closedBy to still trigger
UnknownProp; update the dialog-specific entry in ATTRIBUTE_TAGS_MAP to use
"closedBy" (camelCase) instead of "closedby", and add a corresponding entry in
DOM_ATTRIBUTE_NAMES mapping "closedby" -> "closedBy" so users are guided from
the HTML attribute form to the React prop form and both forms are handled
consistently.
closedby as a valid attribute for <dialog>closedby as a valid attribute for <dialog>
Merging this PR will not alter performance
Comparing Footnotes
|
IA Usage
I made a final check before sending this PR,
invalid.jsx.snap,valid.jsx.snapandfix-closedby-dialog-attribute.mdfiles where updated/added by Claude, the core changes to fix the issue were manually made.Summary
Fixes #9141
Test Plan
Updated
invalid.jsxandvalid.jsxfiles