feat(core): port expo-doctor checks (Expo detection + project-level diagnostics)#583
Conversation
Port expo-doctor's "is this an Expo project?" entry gate. Add an isExpoProject signal to ProjectInfo, keyed off Expo-managed dependencies (expo, expo-router, @expo/cli, …) in the project or any workspace package, mirroring the existing hasReactNativeWorkspace pattern. The expo capability now derives from this signal rather than framework === "expo", so Expo-specific rules load on web-rooted monorepos with an Expo workspace and on projects declaring both expo and a web bundler (where vite/next previously won framework detection). Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
|
React Doctor found 40 files changed in this pull request, but none matched the files covered by its enabled checks. Scope: 40 files changed on Generated by React Doctor. Questions? Contact founders@million.dev. |
…ostics Port the statically-determinable subset of expo-doctor's check suite into react-doctor's environment-checks phase, gated on ProjectInfo.isExpoProject and skipped in diff/staged mode. Adds 11 checks emitting project-level diagnostics (illegal unimodules packages, CLI deps, redundant transitive deps, SDK-critical dependency overrides, expo-router/react-navigation conflict, vector-icons conflict, package.json conflicts, lockfile presence, .expo/.env*.local/local-module gitignore hygiene, and metro config). Each faithfully ports the corresponding expo-doctor check's intent within react-doctor's offline, static model; checks that require running the Expo CLI, the Expo API, or native iOS/Android inspection are out of scope. Extends the PackageJson type with scripts/overrides/resolutions/pnpm/main. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
cursor
Bot
changed the title
…a-driven pass Three of the ported checks (illegal unimodules packages, CLI deps, and redundant transitive deps) performed the identical operation — "is this a direct dependency? (optionally SDK-gated) → warn" — differing only in data. Merge them into a single FLAGGED_DEPENDENCIES table + one filter/map pass, deleting two files and three duplicate loops; per-entry rule keys preserve the user-facing distinction. Extract isExpoSdkAtLeast to replace the repeated null-guarded SDK comparison across three checks. Drop the redundant public isExpoProject function export (matches the unexported RN twin; the discover-project gate imports it directly). Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Read the manifest once and derive directDependencyNames/expoSdkMajor a single time in the aggregator, then pass one uniform ExpoCheckContext to every check. Removes the per-check readPackageJson + re-derivation boilerplate and the mixed (rootDirectory vs derived) implicit conventions, giving all checks one consistent signature. Public checkExpoProject signature and all emitted diagnostics are unchanged (tests untouched). Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
Model Expo like the other runtimes: ProjectInfo now exposes `expoVersion: string | null` (the declared `expo` spec, resolved across the project + workspaces) instead of the `isExpoProject` boolean, paralleling `reactVersion`. `expoVersion !== null` is the is-Expo signal and the SDK major comes straight from it (`expo` major == SDK major), so the checks no longer re-read the manifest for the SDK. Generalize the workspace walk into `findInWorkspacePackageJsons` (a value-returning primitive) and reimplement `someWorkspacePackageJson` on top of it, so the new `findExpoVersion` reuses one walk. Drop the now-unused `is-expo-project`, `is-package-json-expo-aware`, core `isExpoManagedDependencyName`, and the thin `getExpoSdkMajor` wrapper (SDK major parsed inline in the check context). Capability gate keys off `expoVersion !== null`. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
RDE validation resultsNote on harness: the Vercel worker pool was non-functional this session — every repo (including the known-good Setup
Diagnostics by rule (after fixes)
SDK gating verified live: an SDK-54 app with False positives found & fixed (2)
Local checks
|
… positives Two false positives surfaced by validating the ported checks against real Expo OSS projects: - expo-metro-config flagged metro configs that extend Expo's config via a known wrapper (Sentry's getSentryExpoConfig from @sentry/react-native/metro) because the heuristic only matched the literal `expo/metro-config` string. Recognize known extend-signals so wrapper-based configs stay quiet. - expo-router-no-react-navigation fired on SDK 57+ because the port used an unbounded `>=56`, while expo-doctor's ExpoRouterReactNavigationCheck is the closed range `>=56.0.0 <57.0.0`. Add the `<57` upper bound so the check is scoped to the SDK 56 line only. Adds regression tests for both. Co-authored-by: Aiden Bai <aidenybai@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
RDE validation (independent re-run)Re-validated the ported Expo checks against real Expo OSS projects, sampling distinct repos from the eval corpus (
Each diagnostic verified against the manifest:
Bugbot / Devin finding — fixedBoth bots flagged that the "stays quiet on SDK 57+" test called Checks
|
commit: |
| packageJson, | ||
| directDependencyNames: getDirectDependencyNames(packageJson), | ||
| expoSdkMajor: getLowestDependencyMajor(expoVersion), | ||
| }; |
There was a problem hiding this comment.
Expo checks use wrong manifest
Medium Severity
ProjectInfo.expoVersion can come from a workspace package (e.g. apps/mobile), but buildExpoCheckContext always loads dependencies and package.json fields from the scan directory root. Expo diagnostics then run against the wrong manifest while SDK gating uses the workspace expo spec—missing issues in the Expo app and possibly warning on unrelated root dependencies.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 15f6bac. Configure here.
There was a problem hiding this comment.
The project-level Expo checks intentionally operate on the scan-root manifest, consistent with the sibling environment checks (checkReducedMotion, checkPnpmHardening). findExpoVersion resolves the root manifest first, so project.expoVersion already reflects the root when it declares expo (and is now catalog-resolved as of 2a62f78) — in the common single-package / leaf-workspace scan, the SDK and the dependency set come from the same manifest.
The residual case is scanning a web-rooted monorepo at the repo root where expo lives only in a child workspace: the dependency checks then read the root manifest. This is conservative rather than harmful — root-level eas-cli / unimodules / @types/react-native are themselves valid findings, and SDK-gated checks have no expo deps to evaluate against. Full per-workspace manifest analysis when scanning a monorepo root is a documented v1 non-goal (the workspace walk drives the expo capability for file-level lint rules, which do resolve per-package). Leaving open for maintainer input on whether to expand scope.
| packageJson, | ||
| directDependencyNames: getDirectDependencyNames(packageJson), | ||
| expoSdkMajor: getLowestDependencyMajor(expoVersion), | ||
| }; |
There was a problem hiding this comment.
Workspace SDK, root manifest mismatch
Medium Severity
project.expoVersion can come from a child workspace via findExpoVersion, but buildExpoCheckContext always reads dependencies and SDK gating from the scan root’s package.json while still using that workspace-derived version for expoSdkMajor. On a web-rooted monorepo scanned at the repo root, Expo manifest issues in the mobile workspace are missed and SDK-gated rules are applied to the wrong manifest.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 1992b12. Configure here.
There was a problem hiding this comment.
The project-level Expo checks intentionally operate on the scan-root manifest, consistent with the sibling environment checks (checkReducedMotion, checkPnpmHardening). findExpoVersion resolves the root manifest first, so project.expoVersion already reflects the root when it declares expo (and is now catalog-resolved as of 2a62f78) — in the common single-package / leaf-workspace scan, the SDK and the dependency set come from the same manifest.
The residual case is scanning a web-rooted monorepo at the repo root where expo lives only in a child workspace: the dependency checks then read the root manifest. This is conservative rather than harmful — root-level eas-cli / unimodules / @types/react-native are themselves valid findings, and SDK-gated checks have no expo deps to evaluate against. Full per-workspace manifest analysis when scanning a monorepo root is a documented v1 non-goal (the workspace walk drives the expo capability for file-level lint rules, which do resolve per-package). Leaving open for maintainer input on whether to expand scope.
The `inspect()`-driven e2e test drove the full CLI pipeline (config walking, Project service, console/spinner state, git) and was order-dependent under the concurrent suite, failing in CI. Replace it with an isolated `runInspect` harness test (explicit test layers + a real temp manifest for the env checks to read) that covers the same regressions deterministically: the environment-checks phase invokes `checkExpoProject` and surfaces its diagnostics, and the phase is skipped in diff mode. `expoVersion` discovery and per-check logic stay covered by discover-project.test.ts and check-expo-project.test.ts.
| packageJson, | ||
| directDependencyNames: getDirectDependencyNames(packageJson), | ||
| expoSdkMajor: getLowestDependencyMajor(expoVersion), | ||
| }; |
There was a problem hiding this comment.
Workspace SDK mismatches scan manifest
Medium Severity
For a web-rooted monorepo scanned from the repo root, findExpoVersion can supply an expo spec from a workspace such as apps/mobile, while buildExpoCheckContext always loads dependencies and SDK gating from the entry package.json. Dependency-based Expo checks then use the wrong manifest and SDK pairing, causing missed issues in the Expo app and possible spurious warnings on the root package.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0f7ebab. Configure here.
There was a problem hiding this comment.
Same theme as the other expo-check-context threads. The project-level Expo checks intentionally operate on the scan-root manifest, consistent with the sibling environment checks (checkReducedMotion, checkPnpmHardening); findExpoVersion resolves the root first so project.expoVersion reflects the root when it declares expo (now also catalog-resolved). Scanning a web-rooted monorepo at the repo root where expo lives only in a child workspace is conservative (root-level eas-cli/unimodules/@types/react-native are valid findings; SDK-gated checks have no expo deps to evaluate). Per-workspace manifest analysis when scanning a monorepo root is a documented v1 non-goal — leaving open for maintainer input.
| export const checkExpoProject = (rootDirectory: string, project: ProjectInfo): Diagnostic[] => { | ||
| if (project.expoVersion === null) return []; | ||
|
|
||
| const context = buildExpoCheckContext(rootDirectory, project.expoVersion); |
There was a problem hiding this comment.
Expo checks ignore workspace manifest
Medium Severity
When expoVersion comes from a workspace package (e.g. apps/mobile) but the scan root is a web monorepo root, checkExpoProject still builds ExpoCheckContext from the entry package.json and rootDirectory only. SDK-gated dependency rules use the workspace expo spec while reading the root manifest, and filesystem checks skip the Expo app path.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit e96fc81. Configure here.
There was a problem hiding this comment.
Same theme as the other expo-check-context threads. The project-level Expo checks intentionally operate on the scan-root manifest, consistent with the sibling environment checks (checkReducedMotion, checkPnpmHardening); findExpoVersion resolves the root first so project.expoVersion reflects the root when it declares expo (now also catalog-resolved). Scanning a web-rooted monorepo at the repo root where expo lives only in a child workspace is conservative (root-level eas-cli/unimodules/@types/react-native are valid findings; SDK-gated checks have no expo deps to evaluate). Per-workspace manifest analysis when scanning a monorepo root is a documented v1 non-goal — leaving open for maintainer input.
| packageJson, | ||
| directDependencyNames: getDirectDependencyNames(packageJson), | ||
| expoSdkMajor: getLowestDependencyMajor(expoVersion), | ||
| }; |
There was a problem hiding this comment.
Expo checks skip workspace app
Medium Severity
expoVersion is resolved from any workspace package, but buildExpoCheckContext always loads package.json, direct dependencies, and SDK gating from the scan directory only. Analyzing a web-rooted monorepo from its root enables Expo checks while never inspecting the mobile workspace where expo and its problems actually live.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit c8c03b6. Configure here.
There was a problem hiding this comment.
Same theme as the other expo-check-context threads. The project-level Expo checks intentionally operate on the scan-root manifest, consistent with the sibling environment checks (checkReducedMotion, checkPnpmHardening); findExpoVersion resolves the root first so project.expoVersion reflects the root when it declares expo (now also catalog-resolved). Scanning a web-rooted monorepo at the repo root where expo lives only in a child workspace is conservative (root-level eas-cli/unimodules/@types/react-native are valid findings; SDK-gated checks have no expo deps to evaluate). Per-workspace manifest analysis when scanning a monorepo root is a documented v1 non-goal — leaving open for maintainer input.
Replace the temp-dir write-then-read (which depended on the module-level readPackageJson cache during the orchestrated run and flaked in the concurrent CI suite) with a stable committed fixture, and drop the temporary debug probe. The runInspect harness now reads deterministic on-disk content.
Two issues surfaced by review of the ported expo-doctor checks: - `findExpoVersion` returned the raw `expo` spec, so a `catalog:` reference (common in pnpm-catalog monorepos) left `expoVersion` non-null — Expo checks still ran — yet unparseable, silently disabling every SDK-gated rule. Resolve the catalog the same way `react`/`tailwind`/`zod` are. - `checkExpoLockfile` derived the workspace root via `findMonorepoRoot`, which only walks parents; when the scanned project is itself a workspace root it climbed to an outer repo and could falsely report a missing lock file. Prefer the scanned directory when it is itself a monorepo root. Adds regression tests for both.
The runInspect-through-filesystem test was order-dependent in vitest's concurrent worker pool (lazy module loading of the expo-check modules during the orchestrated run intermittently produced empty diagnostics). This is a test-environment artifact only — the published build is a single bundled file with no lazy module resolution. The meaningful regressions remain covered deterministically by check-expo-project.test.ts (every check) and discover-project.test.ts (expoVersion resolution).
Address review findings on the ported expo-doctor checks: - `getExpoDependencySpec` now also reads `expo` from peer/optional dependencies (matching the framework / RN-workspace gates) and guards against a non-string spec, which previously reached `.trim()` and could abort the environment-checks phase on a malformed manifest. - `expo-vector-icons-conflict` now flags any `@react-native-vector-icons/*` scoped package, not only `.../common`, matching the diagnostic wording. - `findInWorkspacePackageJsons` sorts workspace directories so the resolved `expoVersion` (and parsed SDK major) is stable across runs rather than dependent on `readdir` order. Adds regression tests for each.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 7 total unresolved issues (including 5 from previous reviews).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a5f9dab. Configure here.
| } | ||
| } | ||
| } | ||
| expoVersion = resolvedExpoVersion ?? expoVersion; |
There was a problem hiding this comment.
Expo catalog ignores workspace manifest
Medium Severity
When findExpoVersion picks up a catalog: expo spec from a workspace package, catalog resolution only consults the scan-root package.json and parent monorepo pnpm-workspace.yaml, not that workspace’s own catalog / catalogs fields. SDK parsing then fails while expoVersion stays non-null, so SDK-gated Expo rules stay off despite an Expo app in the tree.
Reviewed by Cursor Bugbot for commit a5f9dab. Configure here.
| export const checkExpoMetroConfig = (context: ExpoCheckContext): Diagnostic[] => { | ||
| const metroConfigPath = METRO_CONFIG_FILE_NAMES.map((fileName) => | ||
| path.join(context.rootDirectory, fileName), | ||
| ).find((candidatePath) => isFile(candidatePath)); |
There was a problem hiding this comment.
Expo filesystem checks skip workspaces
Medium Severity
Metro, .env*.local, and .expo / modules/ checks only look under context.rootDirectory. If Expo is detected via a workspace while the scan root is a web monorepo entry, those files under apps/mobile (etc.) are never examined, so real issues are missed even though expo-lockfile already walks to a workspace root.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit a5f9dab. Configure here.
rayhanadev
left a comment
rayhanadev
left a comment
There was a problem hiding this comment.
lgtm, will follow up in another pr with more expo rules based off yesterday's research. feel free to merge
3 participants
What
Ports expo-doctor's check suite into react-doctor. Two parts:
expois present).Part 1 —
expoVersiondetectiondetectFrameworkreturns the first matching package, iterating web bundlers (next,vite, …) beforeexpo, so a project declaring bothexpoand a web bundler — or a web-rooted monorepo with anapps/mobileExpo workspace — silently dropped theexpocapability.ProjectInfo.expoVersion: string | null— the declaredexpopackage spec, resolved across the project and its workspace packages (via a newfindExpoVersion), parallelingreactVersion.expoVersion !== nullis the is-Expo signal, and the Expo SDK major comes straight from it (theexpopackage major tracks the SDK release one-to-one).findInWorkspacePackageJsons(a value-returning primitive);someWorkspacePackageJsonis reimplemented on top of it, so the RN/Reanimated gates andfindExpoVersionall share one walk.buildCapabilitiesderives theexpocapability fromexpoVersion !== nullrather thanframework === "expo".Part 2 — ported checks
Runs in
run-inspect's environment-checks phase (alongsidecheckReducedMotion/checkPnpmHardening), gated onexpoVersion !== nulland skipped in diff/staged mode. The aggregator (check-expo-project.ts) builds a single read-onceExpoCheckContext(root manifest, direct-dependency set, resolved SDK major) and passes it to every check.expo-no-unimodules-packagesexpo-no-cli-dependenciesexpo-no-redundant-dependencyexpo-no-conflicting-dependency-overrideexpo-router-no-react-navigationexpo-vector-icons-conflictexpo-package-json-conflictexpo-lockfileexpo-gitignoregit check-ignoreexpo-env-local-not-gitignoredgit check-ignoreexpo-metro-configThe three pure dependency-presence checks (unimodules / CLIs / redundant transitive deps) share one data-driven
FLAGGED_DEPENDENCIEStable + a single pass; per-entry rule keys preserve the user-facing distinction. SDK-gated checks read the SDK major fromexpoVersionand stay quiet when it can't be resolved, avoiding false positives. The git-dependent checks skip findings when the ignore status is undetermined (no checkout), matching expo-doctor.Intentionally out of scope
The remaining expo-doctor checks require capabilities react-doctor (a static, offline analyzer) doesn't have, so they're not ported:
Type changes
PackageJsonextended withmain,scripts,overrides,resolutions,pnpm.ProjectInfo.expoVersion: string | nulladded (all construction sites updated).Tests
check-expo-project.test.ts— all 11 checks plus the non-Expo gating case (git-backed temp repos for the gitignore/env checks).discover-project.test.ts/build-capabilities.test.ts—expoVersionresolution + capability scenarios.pnpm typecheck,pnpm lint,pnpm format:check,pnpm test(1410 passed / 15 skipped), andpnpm smoke:json-reportall pass.