Fix top React Doctor findings: pnpm hardening, dead files, tooling#196
Merged
Conversation
- pnpm-workspace.yaml: add minimumReleaseAge, trustPolicy, blockExoticSubdeps to narrow the supply-chain window for freshly-published/malicious versions - Delete 18 unreachable files (unused shadcn/ui scaffolding, dead app-header/auth-card/auth-split layout variants never wired into routing) - Verified 2 effect-needs-cleanup findings (MatomoInit.tsx, use-flash-toast.ts) are false positives: router.on() already returns its own unsubscribe function as the effect cleanup, the rule's matcher just doesn't recognize that pattern - Install react-doctor pre-commit hook and CI workflow (advisory-only) for ongoing regression tracking Remaining ~62 lower-severity findings (performance, accessibility, more maintainability) left for a follow-up pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses high-severity React Doctor findings by tightening package-manager supply-chain settings, removing unreachable React/UI files, and adding React Doctor automation in local hooks and CI.
Changes:
- Remove unused React layouts/components and unused shadcn/ui scaffolding files to eliminate
unused-filefindings. - Harden
pnpm-workspace.yamlwith additional security-related settings. - Add React Doctor automation via
package.jsonscript, a pre-commit hook, and a GitHub Actions workflow (plus agent skill docs).
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| resources/js/layouts/auth/auth-split-layout.tsx | Deletes unused auth split layout. |
| resources/js/layouts/auth/auth-card-layout.tsx | Deletes unused auth card layout. |
| resources/js/layouts/app/app-header-layout.tsx | Deletes unused app header layout variant. |
| resources/js/hooks/use-clipboard.ts | Deletes unused clipboard hook. |
| resources/js/components/ui/toggle.tsx | Deletes unused UI component. |
| resources/js/components/ui/toggle-group.tsx | Deletes unused UI component. |
| resources/js/components/ui/placeholder-pattern.tsx | Deletes unused UI component. |
| resources/js/components/ui/navigation-menu.tsx | Deletes unused UI component. |
| resources/js/components/ui/input-otp.tsx | Deletes unused UI component. |
| resources/js/components/ui/icon.tsx | Deletes unused UI component. |
| resources/js/components/ui/dropdown-menu.tsx | Deletes unused UI component. |
| resources/js/components/ui/collapsible.tsx | Deletes unused UI component. |
| resources/js/components/ui/card.tsx | Deletes unused UI component. |
| resources/js/components/ui/badge.tsx | Deletes unused UI component. |
| resources/js/components/ui/alert.tsx | Deletes unused UI component. |
| resources/js/components/feed/MediaBackground.tsx | Deletes unused feed component. |
| resources/js/components/app-header.tsx | Deletes unused app header component. |
| resources/js/components/alert-error.tsx | Deletes unused alert component. |
| pnpm-workspace.yaml | Adds pnpm hardening settings. |
| package.json | Adds doctor script and react-doctor dev dependency. |
| .pre-commit-config.yaml | Adds local react-doctor pre-commit hook. |
| .github/workflows/react-doctor.yml | Adds CI workflow to run React Doctor on PRs and main. |
| .claude/skills/react-doctor/SKILL.md | Adds React Doctor agent skill documentation. |
| .claude/skills/react-doctor/references/explain.md | Adds rule explanation/config guidance for the skill. |
| .agents/skills/react-doctor/SKILL.md | Adds React Doctor agent skill documentation (agents path). |
| .agents/skills/react-doctor/references/explain.md | Adds rule explanation/config guidance (agents path). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+56
to
+58
| - id: react-doctor | ||
| name: react-doctor | ||
| entry: sh -c 'react_doctor_output=$(mktemp "${TMPDIR:-/tmp}/react-doctor-hook.XXXXXX"); if react-doctor --staged --blocking warning > "$react_doctor_output" 2>&1; then rm -f "$react_doctor_output"; else cat "$react_doctor_output" >&2; rm -f "$react_doctor_output"; printf "%s\n" "React Doctor found staged regressions." "Run react-doctor --staged --blocking warning to inspect." "Want them fixed? Ask your agent to run that command and resolve the findings." >&2; fi' |
language: system just shelled out to whatever react-doctor resolved to on PATH — missing entirely on a fresh clone, or silently a different version than package.json's ^0.7.4 devDependency. Switch to language: node with additional_dependencies pinned to react-doctor@0.7.4, so pre-commit installs it into its own isolated env, same fix as the graphify hook on fix/graphify. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The mktemp/if-else entry's exit status was whatever the last command in the branch returned - in the failure branch that was printf, which always succeeds. So the hook printed a warning on regressions but let every commit through regardless of react-doctor's exit code. Replaced with a plain command-substitution + explicit exit 0/1, which also drops the temp-file bookkeeping entirely. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…it.sh Move the check out of the YAML entry string into a real script, matching the existing bin/ convention (dev-server.sh, vite-dev.sh). Same behavior as before, just readable and shellcheck-able instead of a one-line bash -c blob. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No bashisms in the script (just $(), a brace group, and printf/echo), so plain POSIX sh is sufficient. Verified syntax and both exit paths under dash. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…tor version npx react-doctor@latest always fetches whatever's newest instead of the vetted ^0.7.4 devDependency, so npm run doctor could silently run a different (unvetted) version than the rest of the project's tooling. npm resolves devDependency bins automatically, so plain "react-doctor" uses the pinned local install. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.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
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
pnpm-workspace.yamlsupply-chain settings (minimumReleaseAge,trustPolicy,blockExoticSubdeps) to narrow the window for freshly-published/malicious package versionsalert,badge,card,collapsible,dropdown-menu,icon,input-otp,navigation-menu,placeholder-pattern,toggle/toggle-group) plus deadapp-header/auth-card/auth-splitlayout variants never wired into routingeffect-needs-cleanupfindings (MatomoInit.tsx,use-flash-toast.ts) are false positives —router.on()already returns its own unsubscribe function as the effect's cleanup; the linter's matcher just doesn't recognize that pattern. No code change needed.react-doctorpre-commit hook (blocks staged regressions) and CI workflow (advisory-only PR scan + main-branch trend tracking)Remaining ~62 lower-severity findings (performance, accessibility, more maintainability) are left for a follow-up pass.
Test plan
npx tsc --noEmit— no errorsnpx vitest run— 183/183 tests passnpm run build— succeedsnpx react-doctor@latest --verbose— confirmsrequire-pnpm-hardeningandunused-filefindings cleared (80 → 62 total issues)react-doctorhook🤖 Generated with Claude Code