chore: triage-driven rule severity and scope adjustments#536
Merged
Conversation
Next.js App Router treats `opengraph-image.tsx`, `twitter-image.tsx`, `icon.tsx`, and `apple-icon.tsx` (with optional numeric suffix) as metadata image routes. Their default export returns an `ImageResponse` from `next/og` that rasterizes JSX into a static PNG/JPG — no browser DOM, no screen reader, so `alt` / `aria-*` are unactionable noise. Extract `isNextjsMetadataImageRouteFilename` as a shared util (so other DOM-flavoured a11y rules can opt in later) and early-return from `alt-text.create()` when the filename matches. Adds a regression suite covering both extensions, numeric suffixes, and the negative case (`my-opengraph-image.tsx` still flags). Co-authored-by: Cursor <cursoragent@cursor.com>
Mixing non-component exports in a component module is a Fast Refresh hint, not a hard failure — projects routinely co-locate stable constants alongside components and ship fine. Drop from `error` to `warn` so triage output reflects the actual severity gradient. Co-authored-by: Cursor <cursoragent@cursor.com>
The React-Compiler memoization premise didn't hold up for the three canonical hooks the rule targeted: `useRouter`, `useSearchParams`, and `useNavigation` all return stable references, so destructuring their methods produces no measurable compiler win. On Pages Router (`next/router`) destructuring `push` is actively wrong — it captures a stale reference. Introduce a `RULE_IDS_TO_SKIP_REGISTRATION` skiplist in the registry generator (with a required justification comment per entry) so we can park a rule without deleting its implementation. Drops the rule from the registered set (297 → 296), `describe.skip`'s the regression suite, and prunes the rule from the vercel-skill parity table. The fixture seed is kept so re-enabling is a one-line skiplist edit. Co-authored-by: Cursor <cursoragent@cursor.com>
Adjusting derived state inside a useEffect when a prop changes always forces an extra render with a stale UI between the two commits — there is no benign instance of this pattern, so a warning understates the cost. Promote to `severity: "error"` and rewrite the recommendation and message in the authoritative "what's wrong → why → fix" shape used by the rest of the error-level effect rules. Upstream `eslint-plugin-react-you-might-not-need-an-effect` ships this as a softened `type: "suggestion"`. Document the intentional divergence (severity + copy only — detector logic is still a 1:1 port) in `effect/SOURCE.md`, and update the registered-severity assertion in `scan-resilience.test.ts` to expect `error` for this id. Co-authored-by: Cursor <cursoragent@cursor.com>
NisargIO
requested review from
aidenybai and
rayhanadev
and removed request for
aidenybai
Member
|
bugbot run |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8c93b0a. Configure here.
rayhanadev
requested changes
May 28, 2026
rayhanadev
left a comment
rayhanadev
left a comment
Member
There was a problem hiding this comment.
lgtm just small style nits, toss at claude and ask to fix, feel free to merge once done o7
Per @rayhanadev's review: - drop the two `transient-and-async-issues.tsx` block comments around `useRouter` / `LoginLink` (the test files document the rule's intent on their own) - drop the duplicate early-return rationale in `alt-text.ts` (the shared util's header doc already explains the why) - inline the metadata-image basename regex into the helper to remove the named-constant indirection Co-authored-by: Cursor <cursoragent@cursor.com>
After merging main, `RuleContext` no longer exposes `getFilename` (PR \#549 made it `Omit<BaseRuleContext, "getFilename">` and exposed the absolute path as a property). Switch the alt-text early-return to read `context.filename` to match the new API. Co-authored-by: Cursor <cursoragent@cursor.com>
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
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.
Summary
Four independent rule tuning changes from a triage session, each scoped to one commit on the branch:
fix(a11y): skip alt-text on Next.js metadata image route files—opengraph-image.tsx/twitter-image.tsx/icon.tsx/apple-icon.tsx(with optional numeric suffix) rasterize JSX into a static image vianext/og; DOM-flavoured a11y semantics don't apply. Extracts a sharedisNextjsMetadataImageRouteFilenameutil so future rules can opt in, and ships a regression suite for the basenames, extensions, numeric suffixes, and the negative case (my-opengraph-image.tsxstill flags).chore(architecture): demote only-export-components to warn— co-located stable constants are a Fast Refresh hint, not a hard failure. Severity now reflects the actual cost.chore(architecture): unregister react-compiler-destructure-method— the memoization premise didn't hold up for the three hooks it targeted (useRouter,useSearchParams,useNavigation) — all of which return stable references. On Pages Router (next/router), destructuringpushcaptures a stale reference. Adds aRULE_IDS_TO_SKIP_REGISTRATIONskiplist in the registry generator (with a required justification per entry) so we can park a rule without deleting it; the implementation, regression suite (describe.skip'd), and fixture seed all stay so re-enabling is a one-line edit.fix(state-and-effects): promote no-adjust-state-on-prop-change to error— the pattern always forces an extra render with a stale UI between the two commits; there is no benign instance. Promotes to error, rewrites the recommendation + message in the authoritative "what's wrong → why → fix" shape used by sibling error-level effect rules, and documents the intentional divergence from upstream (type: "suggestion") ineffect/SOURCE.md.Test plan
pnpm format:check(auto-formatted the new util)pnpm lint— no new errors; existing fixture warnings unchangedpnpm typecheck— greenvp test runon the touched files:alt-text+alt-text.regressions+only-export-components+no-adjust-state-on-prop-change→ 253 passed, 4 skippedscan-resilience+architecture-rules+vercel-skill-parity→ 55 passed, 3 skipped (thedescribe.skip'dreact-compiler-destructure-methodsuite)Made with Cursor
Note
Medium Risk
Changes default lint severities and stops shipping one rule, which can shift CI pass/fail and diagnostic counts for consumers on default configs.
Overview
This PR tunes lint severity and scope across four areas after triage.
Next.js metadata image routes:
alt-textnow no-ops on App Router files whose basenames matchopengraph-image,twitter-image,icon, orapple-icon(optional numeric suffix, common extensions) via newisNextjsMetadataImageRouteFilename, with regression tests for skip vs. still-flag cases.Architecture:
only-export-componentsis demoted from error → warn (Fast Refresh guidance).react-compiler-destructure-methodis removed from the generated registry through a newRULE_IDS_TO_SKIP_REGISTRATIONskiplist ingenerate-rule-registry.mjswhile keeping source, skipped integration tests, and fixture scaffolding for easy re-enable.State & effects:
no-adjust-state-on-prop-changeis promoted to error with clearer recommendation/message copy;scan-resiliencenow expects that rule aterrorwhile the other seven ported effect rules staywarn, documented ineffect/SOURCE.md.Fixtures and parity tests are updated so
react-compiler-destructure-methodis no longer expected in default scans.Reviewed by Cursor Bugbot for commit 8c93b0a. Bugbot is set up for automated code reviews on this repo. Configure here.