Fix/issue 216 react review audit#238
Conversation
…ect rule The nextjs-no-side-effect-in-get-handler rule was incorrectly flagging response.headers.set() calls as security-risky side effects. These calls only shape the outbound response (Cache-Control, X-Deprecated, CORS headers) and are idiomatic Next.js - they don't mutate server state. This fix adds isResponseHeadersCall() to detect response.headers.set(), response.headers.append(), and response.headers.delete() calls and skip them during side effect detection.
- effect-needs-cleanup: add clearTimeout in animated-score.tsx - nextjs-no-img-element: use next/image in badge-snippet, leaderboard, terminal - nextjs-missing-metadata: add metadata export in page.tsx - async-await-in-loop: add eslint-disable comments for intentional sequential animations - no-inline-exhaustive-style: extract inline styles to constants in og/route.tsx
|
🔴 React Review — 0/100 (unchanged) · No new issues Reviewed by react-review for commit 2c3cbaa. Configure here. |
|
@william-garden is attempting to deploy a commit to the Million Team on Vercel. A member of the Team first needs to authorize it. |
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
|
|
||
| const scoreStyle = { | ||
| fontSize: "120px", | ||
| color: scoreColor, |
There was a problem hiding this comment.
Module-level style objects reference scoreColor and scoreBarPercent variables that are only defined inside the GET function, causing a ReferenceError when the module loads.
![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 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2c3cbaa. Configure here.
| color: scoreColor, | ||
| fontWeight: 700, | ||
| lineHeight: 1, | ||
| }; |
There was a problem hiding this comment.
Module-level styles reference function-scoped dynamic variables
High Severity
The extracted style constants scoreStyle, scoreLabelStyle, and progressFillStyle reference scoreColor and scoreBarPercent at module scope, but these variables are only defined inside the GET handler function (lines 110 and 112). This will crash the OG image route on module load. These styles depend on per-request dynamic values and cannot be hoisted to module-level constants.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 2c3cbaa. Configure here.
| "use client"; | ||
|
|
||
| import { useState } from "react"; | ||
| import Image from "next/image"; |
There was a problem hiding this comment.
Client directive and useState import removed from interactive component
High Severity
The "use client" directive and the useState import from React were removed, but the component still calls useState(false) on line 12 and uses an onClick handler. Without "use client", Next.js treats this as a server component, which cannot use hooks or browser event handlers. This will break the badge snippet's copy functionality entirely.
Reviewed by Cursor Bugbot for commit 2c3cbaa. Configure here.
… in GET handlers (#206) Rewrites `nextjs-no-side-effect-in-get-handler` precision so it no longer floods Next.js codebases with false positives from `response.headers.set()` and request-scoped `new Map/Set/Headers/URLSearchParams/FormData(...)` mutations, while still catching real CSRF-relevant writes: drizzle/prisma ORM mutations, module-level mutable state, mutating fetch, and all three forms of `cookies().set/delete()` including aliased `const cs = await cookies(); cs.set(...)`. Also folds in the safe handler-resolution improvements from PR #251 (cron route skip and depth-bounded `const GET = withAuth(handler)` resolution). Supersedes #209, #211, #233, #238. Co-authored-by: Cursor <cursoragent@cursor.com>
… in GET handlers (#206) Rewrites `nextjs-no-side-effect-in-get-handler` precision so it no longer floods Next.js codebases with false positives from `response.headers.set()` and request-scoped `new Map/Set/Headers/URLSearchParams/FormData(...)` mutations, while still catching real CSRF-relevant writes: drizzle/prisma ORM mutations, module-level mutable state, mutating fetch, and all three forms of `cookies().set/delete()` including aliased `const cs = await cookies(); cs.set(...)`. Also folds in the safe handler-resolution improvements from PR #251 (cron route skip and depth-bounded `const GET = withAuth(handler)` resolution). Supersedes #209, #211, #233, #238. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Working on a related fix in #260 that takes a slightly broader angle for the |
2 participants
fix(#216): resolve React Review audit - 1 error + 8 warnings
Note
Medium Risk
Moderate risk because it changes how
react-doctordetects side effects in GET handlers (security linting), which could alter reported violations; website changes are mostly safe lint/cleanup fixes.Overview
Refines
react-doctorside-effect detection in GET handlers by ignoringresponse.headers.{set,append,delete}()calls (newisResponseHeadersCallhelper) so response-header writes don’t get flagged as server-state mutations.Cleans up Next.js website audit warnings by switching remaining
<img>usages tonext/image, adding missingmetadatato the home page, adding aclearTimeoutcleanup inAnimatedScore, annotating intentionalawait-in-loop animations inTerminal, and extracting OG image inline styles into constants.Reviewed by Cursor Bugbot for commit 2c3cbaa. Bugbot is set up for automated code reviews on this repo. Configure here.