feat: address user feedback — reduce false positives, improve scoring transparency, and add config options#208
Merged
Merged
Conversation
Add 12 AST normalizers that transform minified production code into patterns the react-doctor plugin rules can analyze. Enables 30+ rules to fire on production bundles with 0 parse errors and 0 rule failures across 6 tested sites (vercel, notion, linear, discord, shopify, ami). Normalizers: SequenceExpression callee unwrap, OXC literal type normalization, boolean/void recovery, return/expression sequence splitting, JSX reconstruction (jsx/jsxs/createElement → JSXElement tree with Fragment, ExpressionContainer, key extraction), setter binding + reference rename, and component name uppercase recovery. Also adds parent reference tracking in visitAst, "use client" directive injection, WASM failure caching for CSP-blocked sites, truncated source skip, score calculation, and a null-safety fix for prefer-useReducer. Co-authored-by: Cursor <cursoragent@cursor.com>
… transparency, and add config options - Add `offline`, `designRules`, and `entryFiles` config options - Suppress React 19 deprecation rules on React 18 (migration-hint gate) - Skip `rn-no-raw-text` for `.web.*` files (RN platform convention) - Add sleep/delay and paginated-fetch heuristics to `asyncAwaitInLoop` - Remove `noEmDashInJsxText` rule (em dashes are standard punctuation) - Add `designRules` toggle to disable opinionated design rules - Thread `entryFiles` to knip for dead-code false positive reduction - Export `calculateScoreBreakdown` and show formula in `--verbose` - Document scoring formula, diff/staged modes, and agent integration - Switch to `@changesets/changelog-github` for richer changelogs - Add GitHub Releases workflow Co-authored-by: Cursor <cursoragent@cursor.com>
|
React Review found Copy prompt for agentReviewed by react-review for commit 44ee9ae. Configure here. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![reactreview[bot]](https://avatars.githubusercontent.com/in/2506850?s=60&v=4){kind=small}
| } from "bippy"; | ||
| import { initSync, parseSync } from "@oxc-parser/wasm/web/oxc_parser_wasm.js"; | ||
| import oxcParserWasmBytes from "@oxc-parser/wasm/web/oxc_parser_wasm_bg.wasm"; | ||
| import reactDoctorPlugin from "./plugin/index.js"; |
There was a problem hiding this comment.
Warning
Import from barrel/index file — import directly from the source module for better tree-shaking
Import from the direct path: import { Button } from './components/Button' instead of ./components
Rule: no-barrel-import
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/src/browser-poc.ts">
<violation number="1" location="packages/react-doctor/src/browser-poc.ts:16">
Severity: Warning
Import from barrel/index file — import directly from the source module for better tree-shaking
Import from the direct path: `import { Button } from './components/Button'` instead of `./components`
Rule: `no-barrel-import`
</violation>
</file>
Reviewed by react-review for commit 381b4fb. Configure here.
| const lineNumber = Number(match[2]); | ||
| const columnNumber = Number(match[3]); | ||
| if (!fileName || !Number.isFinite(lineNumber)) continue; | ||
| if (fileName.includes("/node_modules/")) continue; |
There was a problem hiding this comment.
Warning
array.includes() in a loop is O(n) per call — convert to a Set for O(1) lookups
Use a Set or Map for repeated membership tests / keyed lookups — Array.includes/find is O(n) per call
Rule: js-set-map-lookups
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/src/browser-poc.ts">
<violation number="1" location="packages/react-doctor/src/browser-poc.ts:204">
Severity: Warning
array.includes() in a loop is O(n) per call — convert to a Set for O(1) lookups
Use a `Set` or `Map` for repeated membership tests / keyed lookups — `Array.includes`/`find` is O(n) per call
Rule: `js-set-map-lookups`
</violation>
</file>
Reviewed by react-review for commit 381b4fb. Configure here.
| const totalPenalty = errorPenalty + warningPenalty; | ||
| const score = Math.max(0, Math.round(PERFECT_SCORE - totalPenalty)); | ||
| return { | ||
| errorRules: [...errorRules].sort(), |
There was a problem hiding this comment.
Warning
[...array].sort() — use array.toSorted() for immutable sorting (ES2023)
Use array.toSorted() (ES2023) instead of [...array].sort() for immutable sorting without the spread allocation
Rule: js-tosorted-immutable
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/src/utils/calculate-score-locally.ts">
<violation number="1" location="packages/react-doctor/src/utils/calculate-score-locally.ts:62">
Severity: Warning
[...array].sort() — use array.toSorted() for immutable sorting (ES2023)
Use `array.toSorted()` (ES2023) instead of `[...array].sort()` for immutable sorting without the spread allocation
Rule: `js-tosorted-immutable`
</violation>
</file>
Reviewed by react-review for commit 381b4fb. Configure here.
| const score = Math.max(0, Math.round(PERFECT_SCORE - totalPenalty)); | ||
| return { | ||
| errorRules: [...errorRules].sort(), | ||
| warningRules: [...warningRules].sort(), |
There was a problem hiding this comment.
Warning
[...array].sort() — use array.toSorted() for immutable sorting (ES2023)
Use array.toSorted() (ES2023) instead of [...array].sort() for immutable sorting without the spread allocation
Rule: js-tosorted-immutable
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/src/utils/calculate-score-locally.ts">
<violation number="1" location="packages/react-doctor/src/utils/calculate-score-locally.ts:63">
Severity: Warning
[...array].sort() — use array.toSorted() for immutable sorting (ES2023)
Use `array.toSorted()` (ES2023) instead of `[...array].sort()` for immutable sorting without the spread allocation
Rule: `js-tosorted-immutable`
</violation>
</file>
Reviewed by react-review for commit 381b4fb. Configure here.
| const DemoUserCard = ({ userID }: DemoUserCardProps) => { | ||
| const [name, setName] = React.useState("loading"); | ||
|
|
||
| React.useEffect(() => { |
There was a problem hiding this comment.
Warning
fetch() inside useEffect — use a data fetching library (react-query, SWR) or server component
Use useQuery() from @tanstack/react-query, useSWR(), or fetch in a Server Component instead
Rule: no-fetch-in-effect
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/tests/fixtures/browser-poc/app.tsx">
<violation number="1" location="packages/react-doctor/tests/fixtures/browser-poc/app.tsx:17">
Severity: Warning
fetch() inside useEffect — use a data fetching library (react-query, SWR) or server component
Use `useQuery()` from @tanstack/react-query, `useSWR()`, or fetch in a Server Component instead
Rule: `no-fetch-in-effect`
</violation>
</file>
Reviewed by react-review for commit 381b4fb. Configure here.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Replace bespoke flags and filter functions with a unified capabilities + tags system: - Add buildCapabilities(project) that derives a flat Set<string> from ProjectInfo (react:19, nextjs, tanstack-query, etc.) - Add RULE_METADATA map with requires[] (capability gates) and tags (static classification like "design", "test-noise") - Replace filterRulesByReactMajor, filterDesignRules, VERSION_GATED_RULE_IDS, VersionGateMode, and conditional spreads with one shouldEnableRule predicate - Replace 9 individual fields on RunOxlintOptions/OxlintConfigOptions with project: ProjectInfo - Add ignore.tags to user config (replaces designRules boolean) - Cherry-pick from cursor/library-aware-deprecation-rules-ec3b: peerRangeSupportsLegacyReact, isTestFilePath, isLikelyBuildEntry, parseTailwindMajorMinor, isLikelyStringReceiver (js-set-map-lookups fix) - Compute effective React version from min(installed, peerRangeFloor) so library-targeting-legacy is handled by version gating alone Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix Bugbot: peerRangeMinMajor computes the floor major from the peer range so effective version is min(installed, peerFloor) instead of null - Fix Bugbot: replace designRules config key with ignore.tags in README - Add peerRangeMinMajor tests Co-authored-by: Cursor <cursoragent@cursor.com>
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| framework: projectInfo.framework, | ||
| hasReactCompiler: projectInfo.hasReactCompiler, | ||
| hasTanStackQuery: projectInfo.hasTanStackQuery, | ||
| reactMajorVersion: parseReactMajor(projectInfo.reactVersion), |
There was a problem hiding this comment.
The programmatic API diagnose() ignores ignore.tags and entryFiles config options that the CLI correctly handles
Keep capabilities-based architecture, incorporate Tailwind version detection and test file from merged branch. Fix Tailwind null vs unparseable distinction in buildCapabilities. Co-authored-by: Cursor <cursoragent@cursor.com>
- Add auto-suppression in mergeAndFilterDiagnostics: suppress knip/files diagnostics when a matching build artifact exists, suppress test-noise tagged rules in test/fixture files - Tag deprecation and design rules with "test-noise" in RULE_METADATA - Fixes Bugbot: isLikelyBuildEntry is no longer dead code Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…new Promise false negative Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
The loopBodyHasOnlySleepLikeAwaits and hasLoopCarriedDependency checks
were only applied in inspectLoopBody (for/while/do-while) but skipped
for callback-based iteration (.forEach, .map, etc.), causing false
positives on patterns like arr.forEach(async item => { await sleep(500) }).
Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| export const throttledForEach = (items: string[]) => { | ||
| items.forEach(async (item) => { | ||
| await sleep(500); |
There was a problem hiding this comment.
Warning
Async callback in .forEach — return values are dropped, so awaits don't actually wait. Use a for…of loop or await Promise.all(items.map(async (item) => {...}))
Collect the items and use await Promise.all(items.map(...)) to run independent operations concurrently
Rule: async-await-in-loop
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/tests/fixtures/basic-react/src/async-and-handler-issues.tsx">
<violation number="1" location="packages/react-doctor/tests/fixtures/basic-react/src/async-and-handler-issues.tsx:57">
Severity: Warning
Async callback in .forEach — return values are dropped, so awaits don't actually wait. Use a `for…of` loop or `await Promise.all(items.map(async (item) => {...}))`
Collect the items and use `await Promise.all(items.map(...))` to run independent operations concurrently
Rule: `async-await-in-loop`
</violation>
</file>
Reviewed by react-review for commit 37461a8. Configure here.
| export const paginatedForEach = async (cursors: string[]) => { | ||
| let token = "start"; | ||
| cursors.forEach(async () => { | ||
| const page = await fetchPage(token); |
There was a problem hiding this comment.
Warning
Async callback in .forEach — return values are dropped, so awaits don't actually wait. Use a for…of loop or await Promise.all(items.map(async (item) => {...}))
Collect the items and use await Promise.all(items.map(...)) to run independent operations concurrently
Rule: async-await-in-loop
Copy prompt for agent
Check if this React Review issue is valid. If so, understand the root cause and fix it.
Run this before and after your changes to verify the result:
npx react-doctor@latest --verbose --diff
Do not modify the react-doctor configuration unless explicitly asked.
Fix the underlying code issue instead of changing or suppressing the rule.
<file name="packages/react-doctor/tests/fixtures/basic-react/src/async-and-handler-issues.tsx">
<violation number="1" location="packages/react-doctor/tests/fixtures/basic-react/src/async-and-handler-issues.tsx:69">
Severity: Warning
Async callback in .forEach — return values are dropped, so awaits don't actually wait. Use a `for…of` loop or `await Promise.all(items.map(async (item) => {...}))`
Collect the items and use `await Promise.all(items.map(...))` to run independent operations concurrently
Rule: `async-await-in-loop`
</violation>
</file>
Reviewed by react-review for commit 37461a8. Configure here.
| gh release create "$TAG" \ | ||
| --title "$TAG" \ | ||
| --generate-notes \ | ||
| --target main || true |
There was a problem hiding this comment.
GitHub Actions shell injection via direct output interpolation
Low Severity
The ${{ steps.changesets.outputs.publishedPackages }} value is interpolated directly into a run: shell script. If a package name or version in the output contains shell metacharacters (e.g., a single quote), it could break out of the echo '...' command and execute arbitrary shell commands. The safer pattern is to pass the output via an environment variable (e.g., env: PACKAGES: ${{ steps.changesets.outputs.publishedPackages }}) and reference $PACKAGES in the script, which avoids shell interpretation of the value.
Reviewed by Cursor Bugbot for commit 37461a8. Configure here.
Rules from framework-specific maps (NEXTJS_RULES, REACT_NATIVE_RULES, etc.) without a RULE_METADATA entry were unconditionally enabled for all projects. Now they are skipped at runtime, and validateRuleRegistration warns about the gap at dev time. Co-authored-by: Cursor <cursoragent@cursor.com>
Global rules intentionally omit RULE_METADATA entries since they're unconditionally enabled. Extracted FRAMEWORK_SPECIFIC_RULE_KEYS to share the set between the runtime guard and the validation check. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 44ee9ae. Configure here.
| const timings = getTimings(fiber); | ||
| record.selfTime = timings.selfTime; | ||
| record.totalTime = timings.totalTime; | ||
| }; |
There was a problem hiding this comment.
Browser POC overwrites timing instead of accumulating
Low Severity
In collectFiber, selfTime and totalTime are assigned (overwritten) on each call, while instanceCount and commitCount are accumulated with += 1. When multiple fiber instances share the same component type, only the last-visited instance's timing survives. This inconsistency means timing data is silently lost for all but one instance, making the BrowserPocComponentRecord report unreliable aggregated counts but single-instance timings.
Reviewed by Cursor Bugbot for commit 44ee9ae. Configure here.
| const record = getRecord(fiber); | ||
| if (!record) return; | ||
| record.instanceCount += 1; | ||
| record.commitCount += 1; |
There was a problem hiding this comment.
Browser POC commitCount overcounts per fiber instance
Low Severity
record.commitCount += 1 increments once per fiber instance during each tree traversal, not once per commit cycle. A component type with 5 mounted instances increments commitCount by 5 each commit, making the value represent "total instance-visits across all commits" rather than "number of commits involving this component." The field name and interface documentation suggest per-commit granularity.
Reviewed by Cursor Bugbot for commit 44ee9ae. Configure here.
cursor Bot
pushed a commit
that referenced
this pull request
May 12, 2026
…rebase) Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.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
1 participant
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
forwardRef,defaultProps) on React 18 via newmigration-hintversion gate; add sleep/delay and paginated-fetch heuristics toasyncAwaitInLoop; skiprn-no-raw-textfor.web.*files; remove thenoEmDashInJsxTextrule (em dashes are standard punctuation)offline(score locally without API),designRules(toggle opinionated design rules),entryFiles(tell knip about runner entry points to avoid dead-code false positives)calculateScoreBreakdown, show the formula and contributing rules in--verboseoutput, document the scoring formula in README@changesets/changelog-githubfor PR-linked entries, addrelease.ymlworkflow for automatic GitHub Releases on publishTest plan
pnpm typecheckpassespnpm lintpassespnpm format:checkpassespnpm testpasses (706 tests, 51 files)--verboseshows score breakdown on a real projectdesignRules: falsein config suppresses design rules.web.tsxfiles are not flagged byrn-no-raw-textentryFilesconfig forwards to knip correctlyMade with Cursor
Note
Medium Risk
Moderate risk: changes rule enablement/gating and diagnostic filtering (including new auto-suppression heuristics) plus introduces new browser build artifacts and release automation, which could alter lint/score outputs and publishing behavior.
Overview
Adds automated publishing via a new GitHub Actions
releaseworkflow and switches Changesets to@changesets/changelog-githubfor PR-linked changelogs.Reduces noisy diagnostics and makes scoring more controllable: introduces config options for
offlinescoring,ignore.tags(tag-based rule suppression, e.g. design rules), andentryFilesforwarded to knip; refactors oxlint rule selection to a capability/metadata model (React/Tailwind/framework gates) and updates React major detection to respect peer ranges.Improves output transparency and resilience: prints a verbose score formula + contributing rules, exports
calculateScoreBreakdown, auto-suppresses test-noise-tagged rules in test-like paths and suppresses knip “unused file” hits that look like build entry points, and tweaks several rules to reduce false positives (e.g.asyncAwaitInLoop,rn-no-raw-textfor.web.*). Also removes thedesign-no-em-dash-in-jsx-textrule and adds an experimentalbrowser-pocentry/bundle (with wasm loader) for in-browser diagnostics collection.Reviewed by Cursor Bugbot for commit 44ee9ae. Bugbot is set up for automated code reviews on this repo. Configure here.