feat(react-doctor): typed errors + config rootDir for diagnose() targeting#200
Merged
Conversation
Introduces a new errors.ts module exporting ReactDoctorError (base), ProjectNotFoundError, NoReactDependencyError, PackageJsonNotFoundError, and an isReactDoctorError type-guard. The diagnose() entry, scan(), and discoverProject() now throw these typed errors instead of plain Error instances so programmatic API consumers can branch on instanceof / name without parsing message strings. Each error exposes the offending directory as a readable property. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
React Review found Copy prompt for agentReviewed by react-review for commit af22448. Configure here. |
…ProjectError The fallback added in #193 silently picked the first nested React subproject when the requested directory had no root package.json. That worked for the single-subproject case (apps/web with no root manifest) but in true monorepo trees with multiple React projects it would just pick whichever one readdirSync returned first — no signal to the caller, and effectively non-deterministic across filesystems. The CLI surface already has the right ergonomics here via selectProjects: prompt in TTY, scan all under --yes / non-interactive. diagnose() can't reasonably mirror that — it returns a single DiagnoseResult, runs from non-TTY contexts (e.g. the Vercel AI Code Review sandbox), and shouldn't pull prompts into the library bundle. Instead, surface the ambiguity: - 0 nested candidates: ProjectNotFoundError (unchanged) - 1 nested candidate: auto-resolve (preserves the #193 fix) - 2+ nested candidates: throw AmbiguousProjectError with a candidates[] field so callers can iterate or disambiguate. Adds a regression test covering the ambiguous case and exports the new error from the node API. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
A monorepo's only react-doctor config typically lives at the repo root (so editor tooling and child commands all discover it via the standard ancestor walk), but the React app frequently lives in a subfolder like apps/web. Today users must either cd into that subfolder before running react-doctor, pass an explicit path on every invocation, or duplicate the config inside the subproject — and the recently-added AmbiguousProjectError surface makes the silent guess go away but doesn't give them a way to declare the right answer. Add a rootDir config field (mirroring tsconfig's rootDir semantics): when set, the CLI / scan() / diagnose() redirect their target directory to <configDirectory>/<rootDir> (or to rootDir itself when absolute) so a single root-level config can route every invocation at the right project. Resolution rules: - Resolved relative to the config file's directory, not the CWD, so the redirect is stable regardless of where react-doctor is invoked. - Absolute paths used verbatim. - Missing or non-directory paths fall back to the originally requested directory with a logger.warn. - Non-string rootDir values are stripped by validateConfigTypes with the same stderr warning shape used for the boolean fields. - The CLI prints a one-line dimmed redirect notice (suppressed under --json / --score so they remain pure data sinks). Mechanically: - Add rootDir?: string to ReactDoctorConfig. - Refactor load-config to expose loadConfigWithSource (config plus the directory it was loaded from); keep the old loadConfig as a thin wrapper for backward compatibility with the existing 30+ callsites and tests that only need the config. - Add resolve-config-root-dir util that does the resolution + sanity checks and emits the warning on bad input. - Wire the redirect into all three entry points (diagnose, scan, CLI). When the CLI passes configOverride into scan(), scan() trusts that the caller already applied the redirect to avoid double-redirecting. Tests cover: relative + absolute rootDir, ancestor config resolution, disambiguation of an otherwise-AmbiguousProjectError wrapper, non-existent rootDir fallback, and validateConfigTypes stripping a non-string rootDir. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
react-review flagged the [...array].sort() spread allocation on the new test. toSorted() is the ES2023 immutable sort that avoids the intermediate spread copy and is what the js-tosorted-immutable rule asks for. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
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 3568c8c. Configure here.
Bugbot caught that the main scan loop in cli.ts called scan() without configOverride, while the staged-mode and explain-mode loops both passed it. The asymmetry meant scan() would re-load the config from each projectDirectory via the ancestor walk and re-apply the rootDir redirect — so a workspace package nested beneath a redirected target (e.g. apps/web/packages/ui under a root config that sets rootDir: "apps/web") would get redirected back to apps/web and scanned at the wrong directory. Pass configOverride: userConfig from the main loop so scan() trusts the CLI's already-resolved directory and config, matching the other two callsites. Adds two scan() regression tests: - with configOverride: rootDir is NOT re-applied - without configOverride: rootDir IS honored (preserves the programmatic-scan contract from the previous commit) 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
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
Three related changes to how
react-doctorresolves which project to scan, plus the typed error vocabulary that goes with them. Each commit on the branch is independently shippable.1. Typed error classes for the node API
react-doctor/apinow exports named subclasses ofReactDoctorErrorinstead of plainError:ReactDoctorError— base classProjectNotFoundError— no React project (root or nested) under the requested directoryNoReactDependencyError— package found but noreactdepPackageJsonNotFoundError—discoverProject()got a directory with nopackage.jsonAmbiguousProjectError— multiple nested React subprojects under a directory with no rootpackage.json; carriescandidates: readonly string[]isReactDoctorError(value)—value is ReactDoctorErrortype guardEach error preserves its message text (existing message-substring tests still pass) and sets
nameto the class name so it round-trips throughJSON.stringifyanderror.namechecks.2. Refuse ambiguous
diagnose()targetsThe fallback added in #193 silently picked
reactSubprojects[0]when the requested directory had no rootpackage.json. That works for single-subproject roots (the Vercel AI Code Review use case) but in true monorepo trees it just guesses — non-deterministic across filesystems.New behavior:
ProjectNotFoundError(unchanged)AmbiguousProjectErrorwithcandidates[]so the caller iterates or disambiguates explicitlyThe CLI is unaffected — it continues to use
selectProjectsfor prompt-or-scan-all.3.
rootDirinreact-doctor.config.jsonThe declarative answer to "I have a monorepo with
apps/weband want a single root-level config to route every invocation there." Setting"rootDir": "apps/web"inreact-doctor.config.json(or thereactDoctorkey inpackage.json) redirects the CLI /scan()/diagnose()to that subdirectory.Resolution rules:
logger.warn.validateConfigTypeswith the same stderr warning shape used for the boolean fields.Redirected to <relative> via react-doctor config "rootDir"notice (suppressed under--json/--score).This is also the proper escape hatch for the ambiguous-wrapper case from change #2: a repo with no root
package.jsonandapps/web+apps/admincan declare its intent withrootDir: "apps/web"instead of relying on a CLI argument every time.Mechanically: refactored
load-configto addloadConfigWithSource()(config + the directory it loaded from), keptloadConfigas a thin wrapper for backward compatibility, addedutils/resolve-config-root-dir.tsfor the resolution + sanity checks, and wired the redirect into all three entry points. When the CLI passesconfigOverrideintoscan(),scan()trusts that the caller already applied the redirect to avoid double-redirecting.Verification
pnpm typecheck— passpnpm lint— pass (0 warnings, 0 errors on 178 files)pnpm format— cleanpnpm test(inpackages/react-doctor) — 695/695 passrootDirresolution (relative, absolute, ancestor config location, non-existent fallback, non-string validation, source-directory exposure)Example: end-to-end