Skip to content

Commit

Permalink
Stop package.json type field from influencing corepack binaries (#1271)
Browse files Browse the repository at this point in the history
* Stop package.json type field from influencing corepack binaries

This is a small workaround for a side-effect caused by storing corepack installed binaries within the application folder where the `type` field from the application's `package.json` file can affect which module system those binaries are loaded with.

This change adds a dummy `package.json` file to the folder that the corepack binaries are installed into which should prevent the following from happening:

- Node.js will treat the following as ES modules when passed to node as the initial input, or when referenced by import statements or import() expressions
  - Files with a .js extension when the nearest parent package.json file contains a top-level "type" field with a value of "module".
- Node.js will treat the following as CommonJS when passed to node as the initial input, or when referenced by import statements or import() expressions
  - Files with a .js extension when the nearest parent package.json file contains a top-level field "type" with a value of "commonjs".

 Instead, because the dummy `package.json` does not declare a `type` field, the following rule should apply:
 - Node.js defaults to one module system or the other based on the value of the ``--experimental-default-type` flag
   - Files ending in .js or with no extension, if the nearest parent package.json field lacks a "type" field; unless the folder is inside a node_modules folder.

See https://nodejs.org/api/packages.html#determining-module-system.

@fixes #1270
  • Loading branch information
colincasey authored Jun 7, 2024
1 parent 555fe89 commit 1c0e88a
Show file tree
Hide file tree
Showing 13 changed files with 89 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## [Unreleased]

- Fix to stop package.json type field from influencing corepack binaries. ([#1271](https://github.com/heroku/heroku-buildpack-nodejs/pull/1271))

## [v252] - 2024-05-29

Expand Down
7 changes: 7 additions & 0 deletions lib/binaries.sh
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,13 @@ install_corepack_package_manager() {
else
fail_corepack_not_available "$package_manager" "$node_version"
fi

# XXX: Because the corepack binary scripts are located in a sub-directory of the application directory,
# the `type` field from application's package.json can accidentally force an incorrect module
# system from being detected which influences how these binaries scripts are then loaded. Adding the
# following dummy package.json with no `type` set will short-circuit that from happening when Node.js
# runs it's rules for determining the module system.
echo '{ "name": "halt-node-module-system-determination-rules", "version": "0.0.0" }' > "$COREPACK_HOME/package.json"
}

suppress_output() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
This test verifies that the Node.js module resolution rules don't adversely affect
pnpm when installed using corepack.

See:
- https://nodejs.org/api/packages.html#determining-module-system
- https://github.com/heroku/heroku-buildpack-nodejs/issues/1270
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"name": "corepack_pnpm_with_package_type_module",
"packageManager": "[email protected]",
"private": true,
"type": "module",
"engines": {
"node": "^22.1"
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions test/fixtures/corepack_yarn_with_package_type_module/.editorconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
root = true

[*]
end_of_line = lf
insert_final_newline = true

[*.{js,json,yml}]
charset = utf-8
indent_style = space
indent_size = 2
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
/.yarn/** linguist-vendored
/.yarn/releases/* binary
/.yarn/plugins/**/* binary
/.pnp.* binary linguist-generated
13 changes: 13 additions & 0 deletions test/fixtures/corepack_yarn_with_package_type_module/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
.yarn/*
!.yarn/patches
!.yarn/plugins
!.yarn/releases
!.yarn/sdks
!.yarn/versions

# Swap the comments on the following lines if you wish to use zero-installs
# In that case, don't forget to run `yarn config set enableGlobalCache false`!
# Documentation here: https://yarnpkg.com/features/caching#zero-installs

#!.yarn/cache
.pnp.*
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
compressionLevel: mixed

enableGlobalCache: false
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
# yarn_with_package_type_module
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"name": "yarn_with_package_type_module",
"packageManager": "[email protected]"
}
12 changes: 12 additions & 0 deletions test/fixtures/corepack_yarn_with_package_type_module/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 8
cacheKey: 10

"yarn_with_package_type_module@workspace:.":
version: 0.0.0-use.local
resolution: "yarn_with_package_type_module@workspace:."
languageName: unknown
linkType: soft
10 changes: 10 additions & 0 deletions test/run
Original file line number Diff line number Diff line change
Expand Up @@ -1989,6 +1989,16 @@ testPnpmSkipPruning() {
assertCapturedSuccess
}

testCorepackYarnWithPackageTypeModule() {
compile "corepack_yarn_with_package_type_module"
assertCapturedSuccess
}

testCorepackPnpmWithPackageTypeModule() {
compile "corepack_pnpm_with_package_type_module"
assertCapturedSuccess
}

# Utils

pushd "$(dirname 0)" >/dev/null
Expand Down

0 comments on commit 1c0e88a

Please sign in to comment.