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] packages that are in devDependencies AND optionalDependencies are automatically added to package.json's dependencies on npm install #1886

Closed
davidmurdoch opened this issue Oct 1, 2020 · 5 comments · Fixed by npm/arborist#221
Assignees
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release

Comments

@davidmurdoch
Copy link

Current Behavior:

Packages that are in devDependencies AND optionalDependencies are automatically added to package.json's dependencies on npm install.

Expected Behavior:

npm install shouldn't edit the package.json.

npm v6 docs states that Entries in optionalDependencies will override entries of the same name in dependencies, so it’s usually best to only put in one place. (https://docs.npmjs.com/files/package.json#optionaldependencies)

Steps To Reproduce:

  1. npm init -y
  2. npm install noop --save-optional
  3. npm install
  4. view package.json and see that the optional dependency was added to dependencies.

Environment:

  • OS: Kubuntu 20.04
  • Node: 14.13.0
  • npm: 7.0.0-rc.0
@davidmurdoch davidmurdoch added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Oct 1, 2020
@darcyclarke darcyclarke added beta and removed Needs Triage needs review for next steps labels Oct 1, 2020
@darcyclarke darcyclarke added this to the OSS - Sprint 17 milestone Oct 6, 2020
@darcyclarke darcyclarke removed this from the OSS - Sprint 17 milestone Oct 19, 2020
@feross
Copy link

feross commented Oct 28, 2020

I am noticing this same issue on npm 7.0.6, except it happens for packages with are only in optionalDependencies.

@davidje13
Copy link

See also #2203, #724

As noted by feross, the actual bug here is that all optionalDependencies get copied to dependencies. They don't need to also be in devDependencies for this to manifest.

This is a regression in the NPM 7.x line, caused by a large-scale rewrite which performs a mistaken normalisation step when loading the file, then saves its current (now normalised) view back out. Somebody made a PR (npm/normalize-package-data#110) to resolve the optionalDeps -> deps normalisation specifically, but since that normalisation is used in many places it may have other unintended consequences. There is more detailed information in the linked issues.

@vegerot
Copy link

vegerot commented Feb 2, 2021

I am noticing this bug too with optionalDependencies. All of my optionalDependencies are getting moved to dependencies

@ruyadorno ruyadorno self-assigned this Feb 2, 2021
@ruyadorno
Copy link
Contributor

ruyadorno commented Feb 2, 2021

same as: #724

ruyadorno added a commit to ruyadorno/arborist that referenced this issue Feb 2, 2021
Reify currently duplicates entries listed as optionalDependencies in the
users' package.json files. While it's working as expected this is
unexpected to a number of users and it also contradicts our own docs on
it:

    Entries in optionalDependencies will override entries of the same
    name in dependencies, so it's usually best to only put in one place.

This patches this UX problem by adding an extra check that will avoid
adding a dependency to the package.json `dependencies` object in case
that package is already listed under `optionalDependencies`.

Fixes: npm/cli#2203
Fixes: npm/cli#1886
Fixes: npm/cli#724
isaacs pushed a commit to npm/arborist that referenced this issue Feb 4, 2021
Reify currently duplicates entries listed as optionalDependencies in the
users' package.json files. While it's working as expected this is
unexpected to a number of users and it also contradicts our own docs on
it:

    Entries in optionalDependencies will override entries of the same
    name in dependencies, so it's usually best to only put in one place.

This patches this UX problem by adding an extra check that will avoid
adding a dependency to the package.json `dependencies` object in case
that package is already listed under `optionalDependencies`.

Fixes: npm/cli#2203
Fixes: npm/cli#1886
Fixes: npm/cli#724

EDIT(isaacs): Moved this into updateRootPackageJson in pairing session
@xeho91
Copy link

xeho91 commented Mar 16, 2023

Hi, I am not sure whether I should create a new issue, or continue this topic.

I'd like to confirm if this is still an expected behaviour in v9?

In my case, I'd like to have them separated. As in optionalDependencies not being copied to dependencies.
One of the reasons is, in my program I know, these optionalDependencies will not be included in the CI & CD pipeline, because I run the install script with the --omit optional flag. As in:

npm ci --omit optional

While working on a side project, I noticed that a package read-pkg-up which I use to read the nearest package.json file. One of its dependencies uses the normalize-package-data. So, I am receiving false information about what exactly is inside the dependencies, given that the optionalDependencies are being copied here, even when they're not installed (inside the CI & CD pipeline). And it's confusing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release
Projects
None yet
8 participants