fix(nodejs plugin): rewrite setup-corepack as an ESM .mjs module#2857
Open
mikeland73 wants to merge 2 commits into
Open
fix(nodejs plugin): rewrite setup-corepack as an ESM .mjs module#2857mikeland73 wants to merge 2 commits into
mikeland73 wants to merge 2 commits into
Conversation
…:"module" The corepack setup script added in #2845 uses CommonJS `require()`, but was named setup-corepack.js. When a project's root package.json sets `"type": "module"`, Node treats every `.js` file in the tree as an ES module, so running the script crashed the shell with: ReferenceError: require is not defined in ES module scope Rename the script to setup-corepack.cjs (and update the plugin's init_hook and create_files references) so Node always treats it as CommonJS regardless of the project's package.json type. Bump the plugin version 0.0.3 -> 0.0.4. Adds a regression test covering a package.json with "type": "module". Fixes #2856
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a Node.js module-type edge case in the nodejs plugin: when a project’s root package.json sets "type": "module", Node treats setup-corepack.js under .devbox/virtenv/... as ESM and crashes on require(), breaking devbox shell. The fix renames the init hook script to .cjs so Node always executes it as CommonJS.
Changes:
- Renamed the Corepack init hook script to
setup-corepack.cjsto avoid ESM interpretation under"type": "module". - Updated
plugins/nodejs.jsonto reference the new.cjsfilename and bumped plugin version0.0.3→0.0.4. - Added a regression test case covering
"type": "module"inpackage.json.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| plugins/nodejs/setup-corepack.cjs | Updates script header/docs to reflect .cjs usage so CommonJS require() remains valid under ESM projects. |
| plugins/nodejs.json | Points init_hook and create_files at setup-corepack.cjs and bumps plugin version. |
| testscripts/plugin/nodejs_corepack_autodetect.test.txt | Adds a regression case to ensure shells still start when package.json declares "type": "module". |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Rewrites the corepack setup script as a native ES module and names it setup-corepack.mjs. The .mjs extension forces Node to treat it as ESM regardless of the project's package.json "type" field, so it no longer crashes with "require is not defined in ES module scope" when a project sets "type": "module" (issue #2856), and it also works for default CommonJS projects. - require() -> import for child_process and path. - Read package.json via readFileSync + JSON.parse instead of require(), since JSON module import syntax varies across Node versions. - Update the plugin's init_hook and create_files references to the .mjs file. Verified the script exits 0 (no crash) for "type":"module", default/CommonJS, and pinned-packageManager projects.
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
Fixes #2856.
The Corepack setup script introduced in #2845 (
setup-corepack.js) is written in CommonJS and usesrequire(). The nodejs plugin runs it as the shellinit_hook:When a project's root
package.jsonsets"type": "module", Node treats every.jsfile in the directory tree — including this one inside.devbox/virtenv— as an ES module. The script then crashes on its firstrequire(), taking the whole shell down:As a result,
devbox shellno longer starts for any nodejs project whosepackage.jsondeclares"type": "module".Fix
Rewrite the script as a native ES module and name it
setup-corepack.mjs. The.mjsextension forces Node to treat the file as ESM regardless of the surroundingpackage.jsontype, so it works for both"type": "module"projects and default/CommonJS projects. (A plain.jsESM file would break the inverse case — Node parses.jsas CommonJS whentypeis absent orcommonjs.)Changes:
plugins/nodejs/setup-corepack.js→plugins/nodejs/setup-corepack.mjs.require()→importfornode:child_processandnode:path.package.jsonviareadFileSync+JSON.parseinstead ofrequire(), since JSON module import syntax differs across Node versions.init_hookandcreate_filesreferences inplugins/nodejs.json."type": "module"inpackage.json) totestscripts/plugin/nodejs_corepack_autodetect.test.txt.How was it tested?
node --checkon the.mjsfile (valid ESM syntax).package.jsonwith{"type":"module"}— previously the reportedReferenceError.package.json.package.jsonwith a pinnedpackageManager(pnpm@9.0.0) — autodetect path activates correctly.Case 3to the corepack autodetect testscript to guard against regression.cc @AdaptivChris (issue reporter)
Generated by Claude Code