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] Auto install of transitive optional peer if also specified as dev #7772

Closed
2 tasks done
Akatuoro opened this issue Sep 9, 2024 · 3 comments
Closed
2 tasks done
Labels
Bug thing that needs fixing Needs Triage needs review for next steps

Comments

@Akatuoro
Copy link

Akatuoro commented Sep 9, 2024

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

Having a package (peer) both as transitive peerOptional dependency and as dev dependency leads to it being installed for production as well.

Scenario:

optional-peer-test@0.0.1
├─┬ direct@1.0.0 (dependency)
│ └── peer@1.0.0 (peerOptional dependency)
└── peer@1.0.0   (dev dependency)
$ npm i --omit=dev

$ npm explain peer
peer@1.0.0
node_modules/peer
  dev peer@"file:../deps/peer/peer-1.0.0.tgz" from the root project
  peerOptional peer@"file:../peer/peer-1.0.0.tgz" from direct@1.0.0
  node_modules/direct
    direct@"file:../deps/direct/direct-1.0.0.tgz" from the root project

Expected Behavior

The peerOptional dependency is not auto installed for npm i --omit=dev.

Steps To Reproduce

Reproduction repo: https://github.com/Akatuoro/optional-peer-test

For a real world scenario, use the package.json

{
    "name": "optional-peer-test",
    "version": "0.0.1",
    "description": "Testing npm behavior for installing optional peer dependencies",
    "scripts": {
        "test": "npm i --omit=dev && npm ls @sap/hana-client",
        "clean": "rm -Rdf node_modules && rm package-lock.json"
    },
    "dependencies": {
        "@sap/hdi": "4.5.2"
    },
    "devDependencies": {
        "@sap/hana-client": "2.21.31"
    }
}

and run

npm i --omit=dev

resulting in

$ npm ls @sap/hana-client
optional-peer-test@0.0.1
├── @sap/hana-client@2.21.31
└─┬ @sap/hdi@4.5.2
  └── @sap/hana-client@2.21.31 deduped

$ npm explain @sap/hana-client
@sap/hana-client@2.21.31
node_modules/@sap/hana-client
  dev @sap/hana-client@"2.21.31" from the root project
  peerOptional @sap/hana-client@"^2 >= 2.5" from @sap/hdi@4.5.2
  node_modules/@sap/hdi
    @sap/hdi@"4.5.2" from the root project

Also note that in the package-lock.json, the @sap/hana-client is now marked as devOptional, while it was never included under optionalDependencies. When a npm-shrinkwrap.json is then generated and the package is published, the consumer also auto installs the peerOptional dependency.

This seems to be due to arborist not really differentiating between optional dependencies and optional peer dependencies, as e.g. discussed here: #4859 (comment) .

Environment

  • npm: 10.8.3
  • Node.js: v22.7.0
@Akatuoro Akatuoro added Bug thing that needs fixing Needs Triage needs review for next steps labels Sep 9, 2024
@kchindam-infy
Copy link

@Akatuoro The npm i --omit=dev is discussed in the ticket and this is not an issue with ooptional peer dependencies. Please find related info https://github.com/npm/cli/issues/7740.

@Akatuoro
Copy link
Author

@kchindam-infy , thank you for linking the ticket. It's indeed addressing the same issue, but was closed with the argument that ""optional" in peer deps simply means npm will not error if it is unable to be installed".
As ljharb pointed out and according to the documentation, setting optional to true in peerDependenciesMeta means that npm will not automatically install it.
This is also the observed behavior if the dev dependencies are empty. But as soon as the same package is also included in the dev dependencies, it is installed even if --omit=dev is set.

Scenario 1:

    "dependencies": { "@sap/hdi": "4.5.2" }

npm i -> @sap/hana-client is not installed

Scenario 2:

    "dependencies": { "@sap/hdi": "4.5.2" },
    "devDependencies": { "@sap/hana-client": "2.21.31" }

npm i --omit=dev -> @sap/hana-client is installed

This behavior is inconsistent in itself, as adding a dev dependency and omitting it via --omit=dev should not change which packages are installed.
I'm happy to take a shot at fixing this. Should I?

@kchindam-infy
Copy link

Hi @Akatuoro Please feel free to go ahead. Looking forward to your input.

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
Projects
None yet
Development

No branches or pull requests

3 participants