[BUG] Arbitrary package hoisting on NPM workspaces can lead to issues

[Antonio-Laguna]

Is there an existing issue for this?

• I have searched the existing issues

This issue exists in the latest npm version

• I am using the latest npm

Current Behavior

NPM workspaces install dependencies at different levels of the app. When things are required you can get errors because things aren't at the same level.

When root dependency tries to require nested dependency there're issues. While you can often hoist dependencies up this is not always desirable.

It does not work if nested calls to root that tries to find a nested dependency, require.resolve.paths show that cwd isn't included on those paths.

Expected Behavior

require works as long in, either way, no matter where npm decides to install the dependency, or we should be able to set where packages are installed.

See nodejs/node#43429

Steps To Reproduce

1. Need to have workspaces
2. Have dependencies that live in different places (root vs package)
3. A dependency that lives on root that, called from package require a nested package

This is reproducible in this small repository: https://github.com/Antonio-Laguna/node-workspaces-bug

Environment

• npm: 8.11.0
• Node.js: v16.15.1
• OS Name: macOS 13.0
• System Model Name: Macbook Pro
• npm config:

; "user" config from /Users/alaguna/.npmrc

//registry.npmjs.org/:_authToken = (protected)
node_gyp = "usr/local/bin/node-gyp"
tag-version-prefix = ""

; node bin location = /Users/alaguna/.nvm/versions/node/v16.15.1/bin/node
; node version = v16.15.1
; npm local prefix = /Users/alaguna
; npm version = 8.11.0
; cwd = /Users/alaguna
; HOME = /Users/alaguna
; Run `npm config ls -l` to show all defaults.

[ljharb]

require has never been relative to process.cwd(), only to __filename.

[Antonio-Laguna]

That's the issue with workspaces indeed :)

If packages are not at the same level when run there are require issues which seems like a fatal flaw of the workspaces solution? Or perhaps there's something I'm missing?

[ljharb]

I'm confused. A relative require inside a file should only ever be requiring things relative to that file itself, whether using workspaces or not.

In workspaces, from root, a nested package should be required with a bare specifier - ie, require('foo') not require('./packages/foo').

[Antonio-Laguna]

@ljharb that's just to illustrate the point and perhaps is not the best way to portray this.

As the most recent example you might see this PR which is failing WordPress/gutenberg#45697 I can reproduce that the issue (at least locally) is that the new package gets installed within the package node_modules whereas everything else is hoisted to the root which means it can't really be found.

The underlying issue (maybe?) is that packages are (seemingly) arbitrarily hoisted/not-hoisted which can lead to non-deterministic runs

[ljharb]

require also looks upwards in every node_modules folder til it hits /, so hoisting shouldn’t affect anything either.

Apologies I’m on mobile and can’t check out your repro repo; I’m sure it will be a great help to one of the npm folks :-)

[Antonio-Laguna]

So if it wasn't hoisted then it'd be fine, the issue comes when:

• runner is hoisted
  • Task being run (simplification) is not hoisted but nested

In this case runner tries to find the task and fails.

[wraithgar]

This does not seem like an npm bug. This is a bug with a package requiring a dependency not declared in its own package.json. If you want to require a dependency it has to be declared as either a dependency or a peer dependency. Your my-custom-loader package did not declare is-sorted as a dependency so there is no guarantee it will be available.

[Antonio-Laguna]

@wraithgar that is not what's happening here: WordPress/gutenberg#45697 for example you can see that for some reason, the only dependency that's now nested within eslint-plugin/node-modules is eslint-plugin-jsdoc but the package has more:

"@babel/eslint-parser": "^7.16.0",
"@typescript-eslint/eslint-plugin": "^5.3.0",
"@typescript-eslint/parser": "^5.3.0",
"@wordpress/babel-preset-default": "file:../babel-preset-default",
"@wordpress/prettier-config": "file:../prettier-config",
"cosmiconfig": "^7.0.0",
"eslint-config-prettier": "^8.3.0",
"eslint-plugin-import": "^2.25.2",
"eslint-plugin-jest": "^25.2.3",
"eslint-plugin-jsdoc": "^39.6.2",
"eslint-plugin-jsx-a11y": "^6.5.1",
"eslint-plugin-prettier": "^3.3.0",
"eslint-plugin-react": "^7.27.0",
"eslint-plugin-react-hooks": "^4.3.0",
"globals": "^13.12.0",
"requireindex": "^1.2.0"

As I hinted before, that repo might not be the best. I thought it was a bug in Node with require and process.cwd which is why the repo focuses on that. However, I think there's something arbitrary going on at times. It's also not consistent between machines and installs.

In the case of gutenberg right now the structure is as follows:

• scripts has eslint-plugin as dependency. Has no node_modules folder.
• eslint-plugin has all of the dependencies listed above
  • node_modules contains eslint-plugin-jsdoc.

Now when scripts tries to leverage eslint-plugin it can find it since it's on the root. When eslint-plugin tries to require eslint-plugin-jsdoc it can't because the package is on the root but eslint-plugin-jsdoc is on it's own node_modules.

I hope I have painted the issue correctly now and it can be reopened and considered.

[Antonio-Laguna]

@wraithgar @ljharb could this be reopened please?

[noahtallen]

Funny enough, I just came across this exact same issue independently of @Antonio-Laguna in the same repository. I was attempting to update @typescript-eslint/eslint-plugin from v5.3 to v5.62.

Before, require.resolve('@typescript-eslint/eslint-plugin') succeeds from the root. After, it fails. This should be a minor update, and yet Node is arbitrarily not hoisting the package for some unknown reason, causing it to become a breaking update.

To clarify on the explicit dependency point, the dependencies are explicit:

• Repo has a lint script npm run lint:js that references internal script runner contained in packages/scripts.
• packages/scripts (and root package.json) have a dependency on internal monorepo package "@wordpress/eslint-plugin": "file:packages/eslint-plugin",
• packages/eslint-plugin contains the dependency on @typescript-eslint/eslint-plugin
• packages/eslint-plugin contains typescript lint rules.
• Repository contains a .eslintrc.js that that extends rules in @wordpress/eslint-plugin. (Via the explicit root dependency on the internal eslint plugin package.)

At every step of the process, the internal monorepo dependencies are 100% explicit. Even the root .eslintrc.js does not reference @typescript-eslint whatsoever, Eslint fails because the module did not get hoisted. To be even more clear, even though @wordpress/eslint-plugin is explicitly listed as an internal dependency, and even though it has its own explicit dependency on @typescript-eslint/eslint-plugin, and even when the only reference to @typescript/eslint is contained in that package which has an explicit dependency on it, it still fails.

This is either a bug with npm or eslint, unless explicit internal monorepo dependencies aren't supported