Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] workspaces dependencies should take precedence over root transitional dependencies when deduping at the root. #4437

Closed
2 tasks done
mshima opened this issue Feb 18, 2022 · 4 comments
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release

Comments

@mshima
Copy link

mshima commented Feb 18, 2022

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

Converting an existing angular (peer dependency on rxjs@6||7) project with rxjs@7 to monorepository and with concurrently (contains rxjs@6 dependency) at root will make npm install @angular/* peer with rxjs@6 instead of rxjs@7 at the root.

The problem appeared when upgrading to v8.5.0 jhipster/generator-jhipster#17846.
We generate a few independent projects and use the workspace root only for integration scripts (not possible with v8.5.0 anymore due to #4372, now a npm install at a workspace will be applied to the entire monorepository).
The workaround is to add a not required rxjs@7 at the root.

Expected Behavior

package1 rxjs@7 dependency should be moved to the root with @angular/core.

Steps To Reproduce

root package.json:

{
  "workspaces": ["package1"],
  "dependencies": {
    "concurrently": "^7.0.0"
  }
}

package1/package.json:

{
  "dependencies": {
    "@angular/core": "^13.2.3",
    "rxjs": "^7.5.4"
  }
}

npm ls:

workspaces@ /Users/mshima/git/workspaces
├── concurrently@7.0.0
└─┬ package1@ -> ./package1
  ├── @angular/core@13.2.3
  └── rxjs@7.5.4

At root directory:

% ls node_modules
@angular		cliui			date-fns		has-flag		require-directory	strip-ansi		wrap-ansi		zone.js
ansi-regex		color-convert		emoji-regex		is-fullwidth-code-point	rxjs			supports-color		y18n
ansi-styles		color-name		escalade		lodash			spawn-command		tree-kill		yargs
chalk			concurrently		get-caller-file		package1		string-width		tslib			yargs-parser

% cat node_modules/rxjs/package.json| grep version
  "version": "6.6.7",

% ls package1/node_modules
rxjs

% cat package1/node_modules/rxjs/package.json| grep version
  "version": "7.5.4",

Environment

  • npm: 8.5.1
  • Node.js: 16.14.0
  • OS Name: MacOS
  • System Model Name: MacBook Air M1
  • npm config:
; "builtin" config from /opt/homebrew/lib/node_modules/npm/npmrc

prefix = "/opt/homebrew"

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

//registry.npmjs.org/:_authToken = (protected)

; node bin location = /usr/local/bin/node
; cwd = /Users/mshima/git/workspaces
; HOME = /Users/mshima
; Run `npm config ls -l` to show all defaults.
@mshima mshima added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Feb 18, 2022
@mshima mshima changed the title [BUG] npm generates wrong repository tree. [BUG] npm generates wrong workspaces tree. Feb 18, 2022
@mshima mshima changed the title [BUG] npm generates wrong workspaces tree. [BUG] workspaces dependencies should take precedence over root transitional dependencies when deduping at the root. Feb 22, 2022
@nlf
Copy link
Contributor

nlf commented Mar 7, 2022

this behavior is intentional. we prioritize dependency hoisting based on sorting the path to the parent with a locale comparison, which means the root of the project will always come first. we do not intend to change this, as it is the most predictable means of getting a consistent outcome. your resulting package tree is perfectly valid, as every module has access to its own dependencies, which is the requirement when hoisting.

We generate a few independent projects and use the workspace root only for integration scripts (not possible with v8.5.0 anymore due to #4372, now a npm install at a workspace will be applied to the entire monorepository).

you can still access the old behavior, if you cd package1 && npm i --no-workspaces the workspace root detection will be skipped and we'll install dependencies for the workspace directly in the workspace's node_modules.

@nlf nlf closed this as completed Mar 7, 2022
@mshima
Copy link
Author

mshima commented Mar 9, 2022

@nlf how to disable workspaces using .npmrc?

IMO this breaks the workspace peerDependency tree.
If rxjs cannot be moved to the root, so any package that have peer dependency on it should not be moved too.

@ljharb
Copy link
Contributor

ljharb commented Mar 9, 2022

@mshima you can add workspaces=false to .npmrc

@mshima
Copy link
Author

mshima commented Mar 10, 2022

@mshima you can add workspaces=false to .npmrc

@ljharb doesn't work with:

npm ERR! Can not use --no-workspaces and --workspace at the same time

jhipster/generator-jhipster#18081

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release
Projects
None yet
Development

No branches or pull requests

3 participants