optional dependencies installed with --no-optional

cli
priority:medium
triaged

(Gojko Adzic) #1

What I Wanted to Do

install without optional dependencies using --no-optional

What Happened Instead

some optional dependencies were left in node_modules

Reproduction Steps

Create the following package:

{
  "name": "bug-report",
  "version": "1.0.0",
  "description": "",
  "license": "UNLICENSED",
  "private": true,
  "optionalDependencies": {
    "aws-sdk": "^2.316.0"
  }
}

run:

npm install --production --no-optional

node_modules contains ieee754, querystring and sax. As there are no production dependencies, I expected node_modules to be empty or not exist at all

Platform Info

$ npm --versions
{ 'bug-report': '1.0.0',
  npm: '6.4.1',
  ares: '1.10.1-DEV',
  cldr: '32.0',
  http_parser: '2.7.0',
  icu: '60.1',
  modules: '57',
  nghttp2: '1.25.0',
  node: '8.10.0',
  openssl: '1.0.2n',
  tz: '2017c',
  unicode: '10.0',
  uv: '1.19.1',
  v8: '6.2.414.50',
  zlib: '1.2.11' }
$ node -p process.platform
darwin
$ node --version
v8.10.0

npm install --production tries to install devDependencies with node v10.11.0 (npm v6.4.1)
package-lock.json is not complete on first run for some modules.
"npm list errors on 5.6.0 with optional dependencies not installed "
(Lars Willighagen) #2

Reproduced, it seems to install any deduped subdependencies:

039@1.0.0 /home/larsgw/Projects/Testing/npm.community/039
└─┬ UNMET OPTIONAL DEPENDENCY aws-sdk@2.329.0
  β”œβ”€β”¬ UNMET OPTIONAL DEPENDENCY buffer@4.9.1
  β”‚ β”œβ”€β”€ UNMET OPTIONAL DEPENDENCY base64-js@1.3.0
  β”‚ β”œβ”€β”€ ieee754@1.1.8 deduped
  β”‚ └── UNMET OPTIONAL DEPENDENCY isarray@1.0.0
  β”œβ”€β”€ UNMET OPTIONAL DEPENDENCY events@1.1.1
  β”œβ”€β”€ ieee754@1.1.8
  β”œβ”€β”€ UNMET OPTIONAL DEPENDENCY jmespath@0.15.0
  β”œβ”€β”€ querystring@0.2.0
  β”œβ”€β”€ sax@1.2.1
  β”œβ”€β”¬ UNMET OPTIONAL DEPENDENCY url@0.10.3
  β”‚ β”œβ”€β”€ UNMET OPTIONAL DEPENDENCY punycode@1.3.2
  β”‚ └── querystring@0.2.0 deduped
  β”œβ”€β”€ UNMET OPTIONAL DEPENDENCY uuid@3.1.0
  └─┬ UNMET OPTIONAL DEPENDENCY xml2js@0.4.19
    β”œβ”€β”€ sax@1.2.1 deduped
    └── UNMET OPTIONAL DEPENDENCY xmlbuilder@9.0.7

(Lars Willighagen) #3

It’s because of the use of Set here:

Because e.g. ieee745 is required in two ways, aws-sdk -> ieee754 and aws-sdk -> buffer -> ieee754, by the time npm queries the second path aws-sdk is already in seen, and the dependency is automatically marked as non-optional.


(Lars Willighagen) #4

I think this would be fixed by duplicating the seen list at every step. That would still allow checking for circular dependencies without the side effects seen in this case.

diff --git a/lib/install/is-only-optional.js b/lib/install/is-only-optional.js
index 72d6f065e..f1b731578 100644
--- a/lib/install/is-only-optional.js
+++ b/lib/install/is-only-optional.js
@@ -10,6 +10,7 @@ function isOptional (node, seen) {
   if (seen.has(node) || node.requiredBy.length === 0) {
     return false
   }
+  seen = new Set(seen)
   seen.add(node)
   const swOptional = node.fromShrinkwrap && node.package._optional
   return node.requiredBy.every(function (req) {

I’m working on a PR (link) but I’m not sure how to make a test for this, as I can’t reproduce the issue with local dependencies, and online dependencies (such as in the patch) aren’t very future-proof for test cases. Any ideas for that before I submit the PR, @cli-team?

Edit

I had an idea myself (here’s the complete PR), sorry for pinging.