You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
chore: replace import-local and cross-spawn with Node.js built-ins - #4889
webpack-cli used one function from each package. Both are now short code on top of node:module and node:child_process. This removes up to 16 packages from installs: import-local, pkg-dir, find-up, locate-path, p-locate, p-limit, p-try, path-exists, resolve-cwd and resolve-from, plus cross-spawn, path-key, shebang-command, shebang-regex, which and isexe.
import-local → runLocalCLI() in bin/cli.js
It keeps the original logic:
It resolves webpack-cli/bin/cli.js from the real path of the current directory. This is what resolve-cwd did. A missing directory falls back to the resolved path, and other errors skip the lookup.
It runs the local copy only when it is a different file and this one is not inside <cwd>/node_modules. The Windows checks for case and for different drives are kept.
WEBPACK_CLI_SKIP_IMPORT_LOCAL still skips it.
The package name and bin/cli.js path are fixed instead of found through pkg-dir, because this file is always webpack-cli/bin/cli.js.
cross-spawn → #spawn()
getDefaultPackageManager and installPackage are async, so #spawn runs commands with the async child_process.spawn and resolves with the exit status, or with the error when the command fails to start. It doesn't block the way cross-spawn's sync did.
Linux and macOS: the command runs directly, as cross-spawn did.
Windows: the new code follows cross-spawn and which:
It finds the command's file in the current directory, then in PATH, with the extensions from PATHEXT, using fs.promises.stat.
.exe and .com files run directly, without a shell.
Other files, such as npm.cmd, run through cmd.exe /d /s /c "…" (shell: true), with cross-spawn's argument escaping.
.cmd shims in node_modules/.bin pass the arguments on to another shell, so the arguments are escaped twice.
I compared the escaping with cross-spawn's own escape function on 11 inputs, both single and double, and all 22 results were identical. The inputs included quotes, backslashes and %, & and ^.
The command goes in as a single string, which also avoids Node's DEP0190 warning about arguments with shell: true.
Not copied:
ENOENT error:cross-spawn adds one on Windows when a command is missing. The new code doesn't need it, because it treats a non-zero exit status as missing (see below).
Files without an extension:cross-spawn looks these up as a last resort and reads their shebang. Package managers always have a .cmd or .exe file on Windows, so this case doesn't come up here.
Fix: detect missing package managers
spawnSync (and cross-spawn before it) returned a result instead of throwing, so getDefaultPackageManager always chose npm when there was no lock file, even if npm wasn't installed. The yarn and pnpm fallbacks and the "No package manager found." exit never ran. A package manager now counts only when its --version starts without an error and exits with status 0. The probes ignore the output (stdio: "ignore"). This change has its own patch changeset.
Checked with real processes on Linux. With no lock file:
with npm available, it picks npm;
with only pnpm on PATH, it picks pnpm;
with no package manager at all, it prints "No package manager found." and exits with code 2.
Tests
import-local: the test mocked the package in Jest, but the CLI runs in a child process, so the mock never applied. It now installs a fake local webpack-cli in a temporary directory and checks that the CLI runs it, and that WEBPACK_CLI_SKIP_IMPORT_LOCAL skips it. The same tests also pass with the original bin/cli.js.
cross-spawn: the package-manager tests now mock node:child_processspawn with fake processes that exit or fail to start. They check the shell command on Windows. New tests run the Windows branch on any OS and cover a .cmd file, a node_modules/.bin shim and an .exe, plus the fallbacks to yarn and pnpm.
create-webpack-app: the tests now set YARN_IGNORE_ENGINES. The published webpack-cli@7.3.0 depends on rechoir@1, which requires Node.js 22.15, so Yarn 1 failed to install it on Node.js 20.
❌ Patch coverage is 98.13084% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.24%. Comparing base (349b660) to head (7c0b979). ⚠️ Report is 1 commits behind head on main.
It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.
No actionable comments were generated in the recent review.
Recent review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 49b3b7a1-76c9-4c27-b648-e356da9d26a3
Commits
Reviewing files that changed from the base of the PR and between e2f2dcb and 7c0b979.
Files selected for processing: 3
.changeset/detect-missing-package-managers.md
packages/webpack-cli/src/webpack-cli.ts
test/api/install-package.test.js
Files skipped from review as they are similar to previous changes: 3
.changeset/detect-missing-package-managers.md
packages/webpack-cli/src/webpack-cli.ts
test/api/install-package.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.
Walkthrough
The CLI now resolves and delegates to an eligible local webpack-cli without import-local. Package-manager detection and installation use asynchronous Node.js child-process spawning, with Windows-specific executable lookup and argument handling. The package removes import-local and cross-spawn dependencies. Tests cover local CLI delegation, package-manager detection, and platform-specific invocation.
Priority: Normal
Merge Risk: Minimal · up to 7c0b9
Package-manager detection now falls back when a manager is unavailable, and no remaining issue identified here blocks merging.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title "chore: replace import-local and cross-spawn with Node.js built-ins" names the main change. The changeset replaces the import-local and cross-spawn dependencies with Node.js built-ins, which…
Docstring Coverage
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title "chore: replace import-local and cross-spawn with Node.js built-ins" names the main change. The changeset replaces the import-local and cross-spawn dependencies with Node.js built-ins, which the title states. The title is short, specific, and follows a conventional-commit prefix. The changeset also covers the package-manager detection fix and Windows command handling, which the title does not mention, but the title still captures the primary change.
Full details: Docstring Coverage
Explanation
No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 6 files. (1 skipped: 1 unsupported.)
Finishing Touches
📝 Generate docstrings
Commit to this branch
Create a new PR
🧪 Generate unit tests (beta)
Commit to this branch
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Warning
Some tools did not complete. Review the errors below.
ESLint
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
ESLint install timed out. The project may have too many dependencies for the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Test - ubuntu-latest - Node v20.x, Webpack latest, DevServer latest failed in create-webpack-app cli › uses yarn as the package manager when opted:
error rechoir@1.0.0: The engine "node" is incompatible with this module. Expected version ">= 22.15.0". Got "20.20.2"
The generated project installs the published webpack-cli@7.3.0, which depends on rechoir@^1.0.0, and Yarn 1 enforces engine ranges. b7ba111 sets YARN_IGNORE_ENGINES for the generator tests: they check the generated projects, not the engines of the installed packages.
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (1)
Minor · Preserve cross-spawn’s .cmd shim escaping on Windows. · webpack-cli.ts:646-663
packages/webpack-cli/src/webpack-cli.ts:646-663 Minor · Functional Correctness · Quick win
Preserve cross-spawn’s .cmd shim escaping on Windows.
On Windows, when npm, yarn, or pnpm resolves through a node_modules/.bin/*.cmd shim, the helper escapes arguments once before cmd.exe parses the joined command. The shim then parses the forwarded arguments again. Cross-spawn applies a second metacharacter escape for this case.
Without that second escape, a package spec containing &, ", or another cmd metacharacter can be split or corrupted. The package installation can then fail or receive the wrong package spec.
Use cross-spawn’s command resolution and conditional second escape for .cmd shims. Do not apply the second escape unconditionally.
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 84ff2eef-9034-4e5a-9642-54e6ad215bb3
Commits
Reviewing files that changed from the base of the PR and between 1898626 and b7ba111.
Files selected for processing: 1
test/create-webpack-app/test.utils.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
The finding about .cmd shim escaping on Windows (webpack-cli.ts:646-663) is fixed in e76d9ab. #spawnSync now finds the command's file the way cross-spawn and which do: in the current directory first, then in PATH, with PATHEXT.
.exe and .com files run without a shell.
node_modules/.bin/*.cmd shims get the second metacharacter escape.
The escaping gives the same output as cross-spawn's escape function, and tests cover the .cmd, shim and .exe cases.
The reason will be displayed to describe this comment to others. Learn more.
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
⚠️ Outside diff range comments (1)
Minor · Package-manager probes always succeed, even when the tool is missing. · webpack-cli.ts:736
packages/webpack-cli/src/webpack-cli.ts:736 Minor · Functional Correctness · Quick win
Package-manager probes always succeed, even when the tool is missing.
spawnSync does not throw when the executable is missing. It returns a result object, and that object is always truthy. The if (await this.#spawnSync(...)) checks therefore pass on the first probe. The comments above them say an error is thrown, but none is.
When no lockfile exists, getDefaultPackageManager returns "npm" even if npm is not installed.
The yarn and pnpm probes and the "No package manager found." exit are never reached.
On non-Windows platforms, a missing binary shows up as error.code === "ENOENT". On the Windows shell path, a missing command shows up as a non-zero status. A start failure with EACCES or EPERM means the binary exists but could not be launched, so do not report it as "not installed".
cross-spawn also returned a result object, so this behavior existed before. This PR touches these exact lines. Add a helper that checks error and status, then use it in all three probes. Update test/api/get-default-package-manager.test.js to match.
Proposed fix (npm probe shown, repeat for yarn and pnpm)
- if (await this.#spawnSync("npm", ["--version"])) {+ const result = await this.#spawnSync("npm", ["--version"]);++ if (!result.error && result.status === 0) {
return "npm";
}
Based on learnings: check that a spawn failure has code ENOENT before inferring "not installed". Do not treat EACCES/EPERM as missing.
Also applies to: 746-746, 756-756
Source: Learnings
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 08bb82f7-a427-43f1-ac04-1ace33b0e8a1
Commits
Reviewing files that changed from the base of the PR and between b7ba111 and e76d9ab.
Files selected for processing: 3
.cspell.json
packages/webpack-cli/src/webpack-cli.ts
test/api/install-package.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
spawnSync returns a result instead of throwing, so the CLI always chose
npm without a lock file, even when npm was missing. Check the error and
the exit status, so yarn and pnpm are found too.
Claude-Session: https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
The finding that the package-manager probes always succeed (webpack-cli.ts:736, 746, 756) is fixed in bbc02d7. getDefaultPackageManager now tries npm, yarn and pnpm in order and picks the first whose --version runs without an error and exits with status 0. A missing command gives ENOENT, or a non-zero status in the Windows shell. If none passes, it logs "No package manager found." and exits with code 2. An EACCES/EPERM start failure also counts as unavailable, as in your proposed diff: the CLI couldn't install with that package manager either.
The tests now cover falling back to yarn and to pnpm, and a missing package manager is simulated with an ENOENT result instead of a thrown error. 9ce652f adds a patch changeset for the fix.
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Review info
Run configuration
Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: CHILL
Plan: Advanced
Run ID: 2b450a86-c283-456e-82ce-4675b2b210ff
Commits
Reviewing files that changed from the base of the PR and between e76d9ab and e2f2dcb.
Files selected for processing: 4
.changeset/detect-missing-package-managers.md
packages/webpack-cli/src/webpack-cli.ts
test/api/get-default-package-manager.test.js
test/api/install-package.test.js
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review.
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
webpack-cliused one function from each package. Both are now short code on top ofnode:moduleandnode:child_process. This removes up to 16 packages from installs:import-local,pkg-dir,find-up,locate-path,p-locate,p-limit,p-try,path-exists,resolve-cwdandresolve-from, pluscross-spawn,path-key,shebang-command,shebang-regex,whichandisexe.import-local→runLocalCLI()inbin/cli.jsIt keeps the original logic:
webpack-cli/bin/cli.jsfrom the real path of the current directory. This is whatresolve-cwddid. A missing directory falls back to the resolved path, and other errors skip the lookup.<cwd>/node_modules. The Windows checks for case and for different drives are kept.WEBPACK_CLI_SKIP_IMPORT_LOCALstill skips it.The package name and
bin/cli.jspath are fixed instead of found throughpkg-dir, because this file is alwayswebpack-cli/bin/cli.js.cross-spawn→#spawn()getDefaultPackageManagerandinstallPackageare async, so#spawnruns commands with the asyncchild_process.spawnand resolves with the exit status, or with the error when the command fails to start. It doesn't block the waycross-spawn'ssyncdid.cross-spawndid.cross-spawnandwhich:PATH, with the extensions fromPATHEXT, usingfs.promises.stat..exeand.comfiles run directly, without a shell.npm.cmd, run throughcmd.exe /d /s /c "…"(shell: true), withcross-spawn's argument escaping..cmdshims innode_modules/.binpass the arguments on to another shell, so the arguments are escaped twice.cross-spawn's own escape function on 11 inputs, both single and double, and all 22 results were identical. The inputs included quotes, backslashes and%,&and^.shell: true.Not copied:
ENOENTerror:cross-spawnadds one on Windows when a command is missing. The new code doesn't need it, because it treats a non-zero exit status as missing (see below).cross-spawnlooks these up as a last resort and reads their shebang. Package managers always have a.cmdor.exefile on Windows, so this case doesn't come up here.Fix: detect missing package managers
spawnSync(andcross-spawnbefore it) returned a result instead of throwing, sogetDefaultPackageManageralways chosenpmwhen there was no lock file, even ifnpmwasn't installed. Theyarnandpnpmfallbacks and the "No package manager found." exit never ran. A package manager now counts only when its--versionstarts without an error and exits with status0. The probes ignore the output (stdio: "ignore"). This change has its own patch changeset.Checked with real processes on Linux. With no lock file:
npmavailable, it picksnpm;pnpmonPATH, it pickspnpm;Tests
import-local: the test mocked the package in Jest, but the CLI runs in a child process, so the mock never applied. It now installs a fake localwebpack-cliin a temporary directory and checks that the CLI runs it, and thatWEBPACK_CLI_SKIP_IMPORT_LOCALskips it. The same tests also pass with the originalbin/cli.js.cross-spawn: the package-manager tests now mocknode:child_processspawnwith fake processes that exit or fail to start. They check the shell command on Windows. New tests run the Windows branch on any OS and cover a.cmdfile, anode_modules/.binshim and an.exe, plus the fallbacks to yarn and pnpm.create-webpack-app: the tests now setYARN_IGNORE_ENGINES. The publishedwebpack-cli@7.3.0depends onrechoir@1, which requires Node.js 22.15, so Yarn 1 failed to install it on Node.js 20.🤖 Generated with Claude Code
https://claude.ai/code/session_013FhutRFjuQemBzekJMYcqL
Summary by CodeRabbit