fix(a11y/useAnchorContent): don't flag <a> elements used as render props#10220
fix(a11y/useAnchorContent): don't flag <a> elements used as render props#10220theBGuy wants to merge 10 commits into
Conversation
…op values
When a library like BaseUI or shadcn/ui accepts a JSX element via a render
prop (e.g. `render={<a href="..." />}`), the receiving component wraps its
own children inside that element. The final DOM therefore contains both the
anchor's attributes and visible text content, making the lint diagnostic a
false positive.
Adds `is_render_prop_anchor`, which walks the CST upward through the
transparent wrapper nodes (`JsxTagExpression`, `JsxElement`,
`JsParenthesizedExpression`) and returns `true` when the anchor is the direct
value of a `JsxExpressionAttributeValue`. All other positions continue to be
flagged normally.
🦋 Changeset detectedLatest commit: 9a0bfc6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis patch updates the 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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/thick-shoes-jog.md:
- Line 5: Update the changeset note so the referenced rule is linked using the
required website format: replace the plain reference to
lint/a11y/useAnchorContent with the markdown link
[`lint/a11y/useAnchorContent`](https://biomejs.dev/linter/rules/use-anchor-content)
inside .changeset/thick-shoes-jog.md; ensure the link text matches the rule
identifier and the URL points to /linter/rules/use-anchor-content so the
changeset follows the "Include links to rule documentation" guideline.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b6ca24f4-3184-48f9-91d5-0e07409b4853
⛔ Files ignored due to path filters (1)
crates/biome_js_analyze/tests/specs/a11y/useAnchorContent/valid.jsx.snapis excluded by!**/*.snapand included by**
📒 Files selected for processing (3)
.changeset/thick-shoes-jog.mdcrates/biome_js_analyze/src/lint/a11y/use_anchor_content.rscrates/biome_js_analyze/tests/specs/a11y/useAnchorContent/valid.jsx
There was a problem hiding this comment.
Pull request overview
This PR updates the useAnchorContent accessibility lint rule to avoid reporting certain false positives when an <a> element is passed through a JSX attribute as a render-prop-style value, and it aligns the rule docs/tests/release note with that change.
Changes:
- Added an
is_render_prop_anchorancestor walk inuse_anchor_content.rsand documented the new valid pattern in the rule examples. - Added valid JSX fixtures for self-closing, paired, and parenthesized
<a>render-prop forms. - Updated the snapshot and added a patch changeset entry for the lint-rule fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/biome_js_analyze/src/lint/a11y/use_anchor_content.rs | Adds the new attribute-value exemption logic and updates rule documentation. |
| crates/biome_js_analyze/tests/specs/a11y/useAnchorContent/valid.jsx | Adds new valid render-prop anchor fixtures. |
| crates/biome_js_analyze/tests/specs/a11y/useAnchorContent/valid.jsx.snap | Refreshes the valid snapshot to include the new fixtures. |
| .changeset/thick-shoes-jog.md | Adds the release note entry for the patch-level lint fix. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…allowlist
The previous fix exempted any <a> inside a JSX attribute value, which was too
broad — props like `icon={<a />}` would stop reporting even though the anchor
would be rendered with no content.
Narrows the check to verify the attribute name is in an allowlist. The default
list contains only `"render"`. Users can extend it via the new
`additionalRenderProps` option without replacing the default, e.g.:
```
{ "additionalRenderProps": ["as", "component"] }
```
|
Please don't request reviews from copilot. its just extra noise, and we have our own clanker that gives better reviews. also, your PR title was truncated |
Apologies on that one, it happened automatically when I opened the pr and I couldn't get it to stop. |
|
ah, weird. no worries :) |
Merging this PR will not alter performance
Comparing Footnotes
|
…ch release New options on stable rules require a minor release. Split into two PRs: this one (patch, targeting main) keeps just the render prop detection; a follow-up will add the additionalRenderProps option against the next branch. Also replaces the runtime Vec allocation with a const slice for the hard-coded "render" prop name, per review feedback.
This reverts commit 6266f0c. No longer adding the options in this PR
…ponents only - Replace the prop-name allowlist with a component-type check: the rule is now suppressed whenever <a> appears as a JSX attribute value on any custom component (uppercase or member-expression name), not just props named "render". Native HTML elements are unaffected. - Rewrite the helper as `is_component_attribute`, returning `Option<bool>` with the `?` operator instead of the IIFE pattern. - Move the render-prop explanation out of the inline code comment and into prose above the code block in the rule docs.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
crates/biome_js_analyze/src/lint/a11y/use_anchor_content.rs (1)
68-74: ⚡ Quick winAlign the wording with the actual exemption scope.
This now skips any
<a>used as a JSX attribute value on a custom component, not justrender={...}. Keeping “render prop” in the docs and helper name is a bit too specific and will send the next maintainer on a tiny treasure hunt.✏️ Suggested tidy-up
-/// Returns true when the `<a>` element is the value of a JSX attribute that belongs to a -/// custom component (render prop pattern). +/// Returns true when the `<a>` element is the value of a JSX attribute that belongs to a +/// custom component. /// -/// In this pattern the receiving component renders the anchor element as a wrapper and places +/// In this pattern the receiving component may render the anchor element as a wrapper and place /// its own JSX children inside it, so the final DOM will contain both the anchor's attributes /// *and* real text content — making the lint check a false positive. @@ -fn is_render_prop_anchor(node: &AnyJsxElement) -> bool { +fn is_custom_component_attribute_anchor(node: &AnyJsxElement) -> bool {And update the call site at Line 111 to match the rename.
Also applies to: 194-204
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/biome_js_analyze/src/lint/a11y/use_anchor_content.rs` around lines 68 - 74, The docs and helper name currently reference "render prop" but the logic actually exempts any JSX attribute whose value is an anchor element; rename the helper and references from render-prop-specific terms to a generic name like is_jsx_attribute_anchor (update the doc comment above UseAnchorContent examples to remove "render prop" wording and explain the exemption applies to anchor elements used as JSX attribute values on custom components), and update the call site that invokes the helper (the call at the location previously noted around line 111) plus the other occurrences around the 194-204 region to use the new helper name and wording so names and documentation match the actual scope.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/biome_js_analyze/src/lint/a11y/use_anchor_content.rs`:
- Around line 68-74: The docs and helper name currently reference "render prop"
but the logic actually exempts any JSX attribute whose value is an anchor
element; rename the helper and references from render-prop-specific terms to a
generic name like is_jsx_attribute_anchor (update the doc comment above
UseAnchorContent examples to remove "render prop" wording and explain the
exemption applies to anchor elements used as JSX attribute values on custom
components), and update the call site that invokes the helper (the call at the
location previously noted around line 111) plus the other occurrences around the
194-204 region to use the new helper name and wording so names and documentation
match the actual scope.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d27d9b59-1983-452a-b768-4ac939f2af1e
📒 Files selected for processing (1)
crates/biome_js_analyze/src/lint/a11y/use_anchor_content.rs
…on scope Rename `is_render_prop_anchor` → `is_jsx_attribute_anchor` and remove all "render prop" wording from docs and comments. The exemption applies to any JSX attribute on a custom component, not only props named "render".
|
Anything else that needs to be done? |
Verifies that `<a>` inside a JSX attribute on a custom element (hyphenated lowercase name) is still flagged — the exemption only applies to uppercase React components, not web components.
… review - Replace manual parent loop with ancestors().skip(1) - Replace can_cast + unwrap_cast with if let Some(...) = cast(...)
|
@ematipico @dyc3 should I rebase or update with merge commit to resolve the check dependencies action failure, or just not worry about it? |
|
yes, go ahead and update the branch |
Summary
useAnchorContentrule was incorrectly flagging<a>elements passed as render prop values (e.g.render={<a href="..." />}), a common pattern in component libraries such as BaseUI and shadcn/ui where the receiving component renders the anchor as a wrapper around its own children.is_render_prop_anchor, which detects this by walking up the CST throughJsxTagExpression,JsxElement, andJsParenthesizedExpressionuntil it finds aJsxExpressionAttributeValue. If found, the anchor is skipped; any other ancestor causes an immediatefalseso normal bare-<a>violations are unaffected.renderis treated as a render prop name.Test plan
cargo test -p biome_js_analyze use_anchor_content— all 4 tests pass<a />,<a></a>, whitespace-only,aria-hidden) are still reportedrender={<a .../>},render={<a></a>},render={(<a .../>)}) produce no diagnostic