Skip to content

fix(deploy): resolve node_modules by walking up to monorepo root#121

Closed
harrisrobin wants to merge 4 commits intocloudflare:mainfrom
harrisrobin:fix/monorepo-node-modules-resolution
Closed

fix(deploy): resolve node_modules by walking up to monorepo root#121
harrisrobin wants to merge 4 commits intocloudflare:mainfrom
harrisrobin:fix/monorepo-node-modules-resolution

Conversation

@harrisrobin
Copy link
Copy Markdown
Contributor

Problem

vinext deploy fails in monorepos with an ENOENT crash when the wrangler binary is hoisted to the workspace root rather than installed in the app's own node_modules:

Error: spawnSync .../apps/web-next/node_modules/.bin/wrangler ENOENT
    at runWranglerDeploy (dist/deploy.js:622:20)

Three separate places hardcode path.join(root, "node_modules", ...) and fail to find hoisted packages:

  1. detectProjecthasCloudflarePlugin, hasRscPlugin, hasWrangler all check the app-level node_modules only, producing false negatives for all three in a monorepo.
  2. getMissingDepshasMdxRollup has the same issue.
  3. runWranglerDeploy — constructs the wrangler binary path from the app root, crashing with ENOENT when the binary is at the workspace root.

The false negatives in (1) and (2) also cause vinext to try to re-install packages that are already present, adding unnecessary install steps.

Fix

Add findInNodeModules(start, subPath) to utils/project.ts — it walks up parent directories until it finds node_modules/{subPath}, returning the absolute path or null. This mirrors the same approach already used for lock file detection in detectPackageManager.

All four callsites are updated to use it:

// Before
const hasWrangler = fs.existsSync(path.join(root, "node_modules", ".bin", "wrangler"));
const wranglerBin = path.join(root, "node_modules", ".bin", "wrangler");

// After
const hasWrangler = findInNodeModules(root, ".bin/wrangler") !== null;
const wranglerBin = findInNodeModules(root, ".bin/wrangler") ?? path.join(root, "node_modules", ".bin", "wrangler");

Tests

5 new unit tests for findInNodeModules:

✓ finds a package in the immediate node_modules
✓ finds a binary in node_modules/.bin
✓ returns null when not found anywhere
✓ walks up to find package in monorepo root node_modules
✓ prefers the closest node_modules when both app and root have the package

pnpm run typecheck and pnpm run lint pass clean.

/bonk

detectPackageManager only checked for bun.lockb (the old binary lockfile),
missing bun.lock — the text-based format introduced in Bun v1.0. It also
only checked the immediate project directory, causing it to fall back to
npm in monorepo setups where the lock file lives at the workspace root.

Both issues together produce a confusing "npm error Invalid Version:" failure
when running vinext deploy from a Bun monorepo workspace.

Changes:
- Add bun.lock check alongside the existing bun.lockb check
- Walk up parent directories until a lock file is found (same behaviour as
  npm/pnpm/yarn when resolving workspace roots)
- Apply the same two fixes to detectPackageManagerName
- Add unit tests covering: pnpm, yarn, bun.lock, bun.lockb, npm fallback,
  monorepo root traversal, and closest-wins precedence
Node.js fs/promises.glob requires options.exclude to be a function.
Passing an array throws TypeError on Node.js v23 and was never correct
per the API spec (introduced in Node.js v22).

PR cloudflare#52 migrated both routers from the glob npm package to the built-in
but accidentally left array literals in the exclude option for both
app-router.ts and pages-router.ts.

Changes:
- app-router.ts: `exclude: ["**/@*"]` → function that returns true when
  the entry's basename starts with "@"
- pages-router.ts: `exclude: ["api", "**/_*"]` → function that returns
  true when the entry's basename is "api" or starts with "_"

Behaviour is identical — parallel slot directories (@slot) and private
pages (_app, _document, etc.) are still excluded from route scanning.
In monorepos, packages are hoisted to the workspace root's node_modules
rather than installed in each app's own node_modules. vinext was checking
for @cloudflare/vite-plugin, @vitejs/plugin-rsc, wrangler, and @mdx-js/rollup
only in the immediate {root}/node_modules, causing false negatives for all of
them and an ENOENT crash in runWranglerDeploy when the binary was only
present at the workspace root.

Changes:
- Add findInNodeModules(start, subPath) helper to utils/project.ts that
  walks up parent directories until it finds node_modules/{subPath},
  mirroring the same approach used for lock file detection
- Use it in detectProject for hasCloudflarePlugin, hasRscPlugin, hasWrangler
- Use it in getMissingDeps for hasMdxRollup
- Use it in runWranglerDeploy to locate the wrangler binary

Add unit tests covering: direct lookup, binary lookup, null fallback,
monorepo root traversal, and closest-wins precedence.
@harrisrobin
Copy link
Copy Markdown
Contributor Author

Superseded by #122, which combines this with #119 and refactors both into a shared walkUpUntil primitive.

harrisrobin added a commit to harrisrobin/vinext that referenced this pull request Feb 26, 2026
…factored)

Replaces the individual cloudflare#119 and cloudflare#121 fixes with the combined version from
PR cloudflare#122. Introduces walkUpUntil<T> as a shared primitive, with
detectPackageManager and findInNodeModules both built on top of it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant