fix(react): address PR #2024 review feedback

Critical:
- Remove undefined/.claude/session-aliases.json containing __proto__ prototype-pollution
  fixture committed by accident in a7333c14

High:
- agents/react-build-resolver.md: replace brittle `test -o $(grep -l ...)` and
  `test -a -n $(grep ...)` detection with explicit `{ ... || grep -q ...; }` so
  bundler detection no longer breaks when grep returns empty
- agents/react-build-resolver.md: drop hardcoded `npm i react@^19 react-dom@^19`
  remediation; replace with version-agnostic pair-upgrade note that honors the
  project's installed major (17/18/19) — surgical fix principle
- commands/react-review.md: guard `tsc --noEmit -p tsconfig.json` with
  `[ -f tsconfig.json ] &&` so the review skips cleanly on JS-only projects

Medium:
- rules/react/security.md: correct the React-18-blocks-javascript-URL claim
  (React only warns in dev; production navigation is not blocked)
- rules/react/security.md: correct CRA env-var exposure row (CRA exposes
  REACT_APP_*, NODE_ENV, PUBLIC_URL — not 'all' variables)
- skills/react-testing/SKILL.md: instantiate QueryClient once outside the
  wrapper closure so React Query cache survives re-renders (flaky-test fix)
- skills/react-testing/SKILL.md: restore console.error spy with mockRestore()
  in a try/finally so the mock does not leak across tests
- commands/react-test.md: switch outer example-session fence to 4 backticks
  so the inner ```tsx/```bash blocks don't prematurely terminate it
This commit is contained in:
AlexisLeDain
2026-05-21 15:45:47 +02:00
parent f5372f29eb
commit de135f61c7
6 changed files with 29 additions and 33 deletions
+6 -3
View File
@@ -43,8 +43,8 @@ test -f vite.config.js -o -f vite.config.ts -o -f vite.config.mjs # Vite
test -f rsbuild.config.js -o -f rsbuild.config.ts # Rsbuild test -f rsbuild.config.js -o -f rsbuild.config.ts # Rsbuild
grep -l "react-scripts" package.json # CRA grep -l "react-scripts" package.json # CRA
test -f webpack.config.js -o -f webpack.config.ts # webpack test -f webpack.config.js -o -f webpack.config.ts # webpack
test -f .parcelrc -o $(grep -l 'parcel' package.json) # Parcel { test -f .parcelrc || grep -q '"parcel"' package.json; } # Parcel
test -f bunfig.toml -a -n "$(grep '"bun"' package.json)" # Bun { test -f bunfig.toml && grep -q '"bun"' package.json; } # Bun
``` ```
## Diagnostic Commands ## Diagnostic Commands
@@ -160,7 +160,10 @@ Common triggers:
npm ls react # check for duplicates npm ls react # check for duplicates
npm ls @types/react # check version alignment npm ls @types/react # check version alignment
npm dedupe # consolidate duplicates npm dedupe # consolidate duplicates
npm i react@^19 react-dom@^19 # upgrade as a pair, never independently # Only when `npm ls react` reports duplicates or a version mismatch with `@types/react`.
# Upgrade react and react-dom as a pair (matching the major already in use) — never independently.
# Replace <major> with the project's React major (17 / 18 / 19); jumping majors is a separate, deliberate change.
# npm i react@^<major> react-dom@^<major>
``` ```
When a library throws on hook usage, it almost always means React is duplicated. When a library throws on hook usage, it almost always means React is duplicated.
+1 -1
View File
@@ -84,7 +84,7 @@ npx eslint . --ext .tsx,.jsx,.ts,.js
# Typecheck (skip cleanly for JS-only projects) # Typecheck (skip cleanly for JS-only projects)
npm run typecheck --if-present npm run typecheck --if-present
tsc --noEmit -p tsconfig.json [ -f tsconfig.json ] && tsc --noEmit -p tsconfig.json
# Targeted a11y rules # Targeted a11y rules
npx eslint . --rule 'jsx-a11y/alt-text: error' \ npx eslint . --rule 'jsx-a11y/alt-text: error' \
+2 -2
View File
@@ -45,7 +45,7 @@ Prefer Vitest for new Vite-based projects; respect Jest for existing setups.
## Example Session ## Example Session
```text ````text
User: /react-test I need a SearchInput component with debounced search User: /react-test I need a SearchInput component with debounced search
Agent: Agent:
@@ -173,7 +173,7 @@ $ vitest run --coverage src/components/SearchInput.test.tsx
``` ```
## TDD Complete! ## TDD Complete!
``` ````
## Test Patterns ## Test Patterns
+1 -1
View File
@@ -108,7 +108,7 @@ Prefixed env vars are bundled into the client. Treat them as public.
|---|---|---| |---|---|---|
| Next.js | `NEXT_PUBLIC_*` | All others | | Next.js | `NEXT_PUBLIC_*` | All others |
| Vite | `VITE_*` | `.env` server-side only | | Vite | `VITE_*` | `.env` server-side only |
| Create React App | `REACT_APP_*` | None — CRA exposes all | | Create React App | `REACT_APP_*`, plus `NODE_ENV` and `PUBLIC_URL` | All others (anything without the `REACT_APP_` prefix is server-side only) |
| Remix | `process.env` access in `loader`/`action` only | Same | | Remix | `process.env` access in `loader`/`action` only | Same |
```ts ```ts
+13 -4
View File
@@ -203,8 +203,13 @@ test("useCounter accepts initial value", () => {
}); });
test("useUser fetches user data", async () => { test("useUser fetches user data", async () => {
// Instantiate QueryClient ONCE per test outside the wrapper so it survives re-renders.
// Creating it inside the wrapper closure resets cache state on every render, producing flaky tests.
const queryClient = new QueryClient({
defaultOptions: { queries: { retry: false } },
});
const wrapper = ({ children }: { children: React.ReactNode }) => ( const wrapper = ({ children }: { children: React.ReactNode }) => (
<QueryClientProvider client={new QueryClient()}>{children}</QueryClientProvider> <QueryClientProvider client={queryClient}>{children}</QueryClientProvider>
); );
const { result } = renderHook(() => useUser("1"), { wrapper }); const { result } = renderHook(() => useUser("1"), { wrapper });
@@ -385,9 +390,10 @@ function Broken() {
} }
test("error boundary renders fallback", () => { test("error boundary renders fallback", () => {
// Suppress React's console.error noise for expected throw // Suppress React's console.error noise for the expected throw, then restore so
vi.spyOn(console, "error").mockImplementation(() => {}); // the spy does not leak across tests and hide real errors elsewhere.
const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {});
try {
render( render(
<ErrorBoundary fallback={<div>Something went wrong</div>}> <ErrorBoundary fallback={<div>Something went wrong</div>}>
<Broken /> <Broken />
@@ -395,6 +401,9 @@ test("error boundary renders fallback", () => {
); );
expect(screen.getByText("Something went wrong")).toBeInTheDocument(); expect(screen.getByText("Something went wrong")).toBeInTheDocument();
} finally {
errorSpy.mockRestore();
}
}); });
``` ```
-16
View File
@@ -1,16 +0,0 @@
{
"version": "1.0.0",
"aliases": {
"__proto__": {
"sessionPath": "/evil/path",
"createdAt": "2026-05-20T09:16:26.009Z",
"title": "Prototype Pollution Attempt"
},
"normal": {
"sessionPath": "/normal/path",
"createdAt": "2026-05-20T09:16:26.009Z",
"title": "Normal Alias"
}
},
"metadata": { "totalCount": 2, "lastUpdated": "2026-05-20T09:16:26.009Z" }
}