shrinkwrap + prune + npm3 = packages being re-downloaded

[fiznool]

Currently, when building the dependencies, the buildpack runs npm prune and then npm install.

npm3 has changed to installing dependencies 'maximally flat', i.e. npm has tried to pull out as many nested dependencies as possible, and put them at the top level in the node_modules dir.

As a result, when subsequently running npm shrinkwrap, you'll potentially end up with a shrinkwrap file with lots of 'top-level' dependencies that don't necessarily correspond to what is described in package.json.

npm prune doesn't take into account the shrinkwrap at all - it just looks in the package.json and uninstalls any top level packages that are in the node_modules folder, but not in package.json.

So you can see there is an issue:

• A node_modules cache previously installed by a maximally flat shrinkwrap won't really correspond to a package.json.
• Running npm prune will remove many top level dependencies that aren't in package.json.
• Running npm install will (probably) re-download many that were removed by npm prune.

Obviously this isn't really desirable, and somewhat negates the benefits of the node_modules cache.

[hunterloftis]

Thanks for the issue and description!

The reason npm prune runs before npm install is to remove packages that were previously installed but no longer used. Otherwise, throughout the life of an application, dozens of unnecessary dependencies can build up in the cache.

It'd be great to discover whether npm prune ignoring shrinkwrap is desired behavior, or a bug, moving forward into the flattened world of npm 3 /cc @othiym23

[othiym23]

1. npm prune should take the shrinkwrap into account.
2. It should be an error for the shrinkwrap and package.json to conflict.

That both aren't true right now are among the many places where shrinkwrap is underspecified around the edges (see also the discussions about how best to handle optionalDependencies and devDependencies in shrinkwraps).

So yes, there are bugs, or maybe just holes in npm's functionality here. We're in the process of shoring up npm's tests for the rest of the year, so we're not going to have time to fix any bugs filed around this for a while, but I would be completely behind somebody taking a stab at making orune's behavior more consistent here.

That said, the initial description of the problem isn't complete – the behavior described should happen only with trees initially installed with npm@2 and then subsequently switched to being managed with npm@3. npm@3 writes new metadata into the installed package.json files that allow it to determine how and why a package was installed (among other things, that's how npm ls is able to reconstruct the logical shape of a physically flattened install tree). npm prune operates on these logical trees, so a tree installed with npm@3 from the outset shouldn't unnecessarily remove files.

I'm not, however, completely sure how the presence of a shrinkwrap might change how that metadata gets added, and there in fact might be other bugs around the interaction of shrinkwraps and various other commands with npm@3. They are bugs, though, and not the intended behavior, so fixes would be gratefully accepted.

[hunterloftis]

Thanks @othiym23!

I'll close this since we try to resolve root issues rather than work around them at the buildpack level. At the moment I typically recommend using --save --save-exact in lieu of shrinkwrap (due to the aforementioned holes in shrinkwrap's current behavior around the edges).

[fiznool]

@hunterloftis unfortunately --save-exact isn't really the best option, since nested dependencies are not locked down. Only shrinkwrap does this.

Personally, I can imagine that there will be many Heroku apps that upgrade from npm2 > 3, and so will see this issue. I'll try to do some further testing, but IMO the buildpack is running prune prior to install, which is exhibiting the above behaviour, so this issue should remain open until the underlying problem is fixed.

[hunterloftis]

Personally, I can imagine that there will be many Heroku apps that upgrade from npm2 > 3, and so will see this issue.

When you change versions of node or npm, the buildpack invalidates your node_modules cache, so there will be nothing to prune:

$ git commit -am 'npm 2'

$ git push heroku master
Counting objects: 6, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 538 bytes | 0 bytes/s, done.
Total 6 (delta 4), reused 0 (delta 0)
remote: Compressing source files... done.
remote: Building source:
remote:
remote: -----> Using set buildpack heroku/nodejs
remote: -----> Node.js app detected
remote:
remote: -----> Creating runtime environment
remote:
remote:        NPM_CONFIG_LOGLEVEL=error
remote:        NPM_CONFIG_PRODUCTION=true
remote:        NODE_ENV=production
remote:        NODE_MODULES_CACHE=true
remote:
remote: -----> Installing binaries
remote:        engines.node (package.json):  0.12.7
remote:        engines.npm (package.json):   2.x
remote:
remote:        Downloading and installing node 0.12.7...
remote:        Resolving npm version 2.x via semver.io...
remote:        Downloading and installing npm 2.14.13 (replacing version 2.11.3)...
remote:
remote: -----> Restoring cache
remote:        Loading 2 from cacheDirectories (default):
remote:        - node_modules
remote:        - bower_components (not cached - skipping)
remote:
remote: -----> Building dependencies
remote:        Pruning any extraneous modules
remote:        Installing node modules (package.json)
remote:
remote: -----> Caching build
remote:        Clearing previous node cache
remote:        Saving 2 cacheDirectories (default):
remote:        - node_modules
remote:        - bower_components (nothing to cache)
remote:
remote: -----> Build succeeded!
remote:        ├── body-parser@1.12.0
remote:        ├── cookie-parser@1.3.4
remote:        ├── debug@2.1.1
remote:        ├── express@4.12.2
remote:        ├── jade@1.9.2
remote:        ├── morgan@1.5.1
remote:        └── serve-favicon@2.2.0

$ git commit -am 'npm 3'

$ git push heroku master
Counting objects: 3, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (3/3), done.
Writing objects: 100% (3/3), 288 bytes | 0 bytes/s, done.
Total 3 (delta 2), reused 0 (delta 0)
remote: Compressing source files... done.
remote: Building source:
remote:
remote: -----> Using set buildpack heroku/nodejs
remote: -----> Node.js app detected
remote:
remote: -----> Creating runtime environment
remote:
remote:        NPM_CONFIG_LOGLEVEL=error
remote:        NPM_CONFIG_PRODUCTION=true
remote:        NODE_ENV=production
remote:        NODE_MODULES_CACHE=true
remote:
remote: -----> Installing binaries
remote:        engines.node (package.json):  0.12.7
remote:        engines.npm (package.json):   3.x
remote:
remote:        Downloading and installing node 0.12.7...
remote:        Resolving npm version 3.x via semver.io...
remote:        Downloading and installing npm 3.5.0 (replacing version 2.11.3)...
remote:
remote: -----> Restoring cache
remote:        Skipping cache restore (new runtime signature)
remote:
remote: -----> Building dependencies
remote:        Pruning any extraneous modules
remote:        Installing node modules (package.json)
remote:        express@0.0.0 /tmp/build_1213013796e8c72036b0c2b708bd85d4
remote:        ├─┬ body-parser@1.12.0
remote:        │ ├── bytes@1.0.0
remote:        │ ├── content-type@1.0.1
remote:        │ ├── depd@1.0.1
remote:        │ ├── iconv-lite@0.4.7
remote:        │ ├─┬ on-finished@2.2.1
remote:        │ │ └── ee-first@1.1.0
remote:        │ ├── qs@2.3.3
remote:        │ ├── raw-body@1.3.3
remote:        │ └─┬ type-is@1.6.9
remote:        │   ├── media-typer@0.3.0
remote:        │   └─┬ mime-types@2.1.8
remote:        │     └── mime-db@1.20.0
remote:        ├─┬ cookie-parser@1.3.4
remote:        │ ├── cookie@0.1.2
remote:        │ └── cookie-signature@1.0.6
remote:        ├─┬ debug@2.1.1
remote:        │ └── ms@0.6.2
remote:        ├─┬ express@4.12.2
remote:        │ ├─┬ accepts@1.2.13
remote:        │ │ └── negotiator@0.5.3
remote:        │ ├── content-disposition@0.5.0
remote:        │ ├── escape-html@1.0.1
remote:        │ ├─┬ etag@1.5.1
remote:        │ │ └── crc@3.2.1
remote:        │ ├── finalhandler@0.3.3
remote:        │ ├── fresh@0.2.4
remote:        │ ├── merge-descriptors@1.0.0
remote:        │ ├── methods@1.1.1
remote:        │ ├── parseurl@1.3.0
remote:        │ ├── path-to-regexp@0.1.3
remote:        │ ├─┬ proxy-addr@1.0.8
remote:        │ │ ├── forwarded@0.1.0
remote:        │ │ └── ipaddr.js@1.0.1
remote:        │ ├── range-parser@1.0.3
remote:        │ ├─┬ send@0.12.1
remote:        │ │ ├── destroy@1.0.3
remote:        │ │ ├── mime@1.3.4
remote:        │ │ └── ms@0.7.0
remote:        │ ├─┬ serve-static@1.9.3
remote:        │ │ └─┬ send@0.12.3
remote:        │ │   ├── debug@2.2.0
remote:        │ │   ├── etag@1.6.0
remote:        │ │   └── ms@0.7.1
remote:        │ ├── utils-merge@1.0.0
remote:        │ └── vary@1.0.1
remote:        ├─┬ jade@1.9.2
remote:        │ ├── character-parser@1.2.1
remote:        │ ├── commander@2.6.0
remote:        │ ├─┬ constantinople@3.0.2
remote:        │ │ └── acorn@2.6.4
remote:        │ ├─┬ mkdirp@0.5.1
remote:        │ │ └── minimist@0.0.8
remote:        │ ├─┬ transformers@2.1.0
remote:        │ │ ├─┬ css@1.0.8
remote:        │ │ │ ├── css-parse@1.0.4
remote:        │ │ │ └── css-stringify@1.0.5
remote:        │ │ ├─┬ promise@2.0.0
remote:        │ │ │ └── is-promise@1.0.1
remote:        │ │ └─┬ uglify-js@2.2.5
remote:        │ │   ├─┬ optimist@0.3.7
remote:        │ │   │ └── wordwrap@0.0.3
remote:        │ │   └─┬ source-map@0.1.43
remote:        │ │     └── amdefine@1.0.0
remote:        │ ├── void-elements@2.0.1
remote:        │ └─┬ with@4.0.3
remote:        │   ├── acorn@1.2.2
remote:        │   └── acorn-globals@1.0.9
remote:        ├─┬ morgan@1.5.1
remote:        │ └── basic-auth@1.0.0
remote:        └─┬ serve-favicon@2.2.0
remote:        └── ms@0.7.0
remote:
remote:
remote: -----> Caching build
remote:        Clearing previous node cache
remote:        Saving 2 cacheDirectories (default):
remote:        - node_modules
remote:        - bower_components (nothing to cache)
remote:
remote: -----> Build succeeded!
remote:        ├── body-parser@1.12.0
remote:        ├── cookie-parser@1.3.4
remote:        ├── debug@2.1.1
remote:        ├── express@4.12.2
remote:        ├── jade@1.9.2
remote:        ├── morgan@1.5.1
remote:        └── serve-favicon@2.2.0

[fiznool]

@hunterloftis of course. Silly me.

So basically this is a bug with how shrinkwrap works, so should be tracked upstream. That's fair enough. Thanks for your comments!

[hunterloftis]

@fiznool would the buildpack skipping the prune step if it detected a shrinkwrap file help your use case? You'd probably want to periodically disable the cache to clear out old / unused packages.

[fiznool]

Yes, that would be a useful workaround. Can this be achieved for npm3 only?

[hunterloftis]

@fiznool it could be, though I hesitate to put in version-specific workarounds (over time the complexity and backwards compatibility requirements really add up, and I have to support them).

For now let's test a branch that simply doesn't prune dependencies. Long-term the buildpack will need to prune dependencies occasionally; this could be done by caching the number of unpruned builds and then pruning every X builds... or more ideally npm-next will make prune compatible with the flattened dep tree + shrinkwrap and we can remove this workaround:

$ heroku buildpacks:set https://github.com/heroku/heroku-buildpack-nodejs#no-prune