Check dependency versions before running any commands

[domenic]

a.k.a., "put an end to self-trolling."

This is a very common experience when working on teams of size larger than one:

• The team has decided to .gitignore their node_modules.
• Team Member A updates foo, e.g. npm install foo --save.
• Team Member A commits the resulting package.json and pushes to Git.
• Team Member B finishes all their work on their current feature, and is ready to rebase.
• They do so, but, now all the tests are failing! Or perhaps they aren't failing, but the app is acting weird.
• After a half-hour of sadness and frustrating debugging, Team Member B finally figures out they forgot to run npm install to pick up the latest changes.

Proposal: before any operations involving npm, including e.g. npm test, npm checks to see if all versions match the ones declared in package.json.

Complexities:

• Ideally this should be multi-level, i.e. recursing and doing semver matches.
• If a shrinkwrap file is present, this should be able to do exact version matching against the shrinkwrap file at every level in the tree.
• It's unclear how much overlap there is between this code and the validation npm ls already performs. I suspect npm ls code does not deal with shrinkwrap well, and is not as fast as it could be (e.g. I think it does normalization). But it might be a start.

The minimum viable version of this for Ember is fixing #5083 plus making this work for shrinkwrap.

/cc @stefanpenner

[stefanpenner]

An additional and common scenario, is too accidentally forget the --save. Now your coworkers have no idea the version you depended on, and you have caused grief.

[derekprior]

That still requires interaction with the npm command line interface. What if there was runtime support (optional) for the package.json and npm-shrinkwrap.json files? That is, perhaps I could require('npm-safety')(); and when the app starts the dependency version would be checked against those?

[domenic]

@derekprior I think that's a good idea in general---factoring out the check into a separate package would be a good way to divide the work inside npm, and make it reusable. But in the end you should be starting your app with npm start, which would take care of that automatically.

This will become an even better safety net once npm exec gets implemented (#1543)

[Raynos]

@derekprior we are thinking of adding this feature in https://github.com/uber/npm-shrinkwrap as a postinstall hook.

There are a bunch of subtleties here.

You want to verify that

• node_modules
• npm-shrinkwrap.json
• package.json

All agree about what the versions of dependencies are. This can introduce complexities with ranges & git dependencies.

If you use ranges & git dependencies (without shas) at best we can do is verify a git / npm range in package.json matches node_modules.

For verifying npm-shrinkwrap & node_modules we should be able to check resolved agrees recursively.

For branches & tags in package.json we can either

• a branch in package.json is always valid with node_modules because the branch could have pointed at anything
• a branch in package.json is valid with node_modules, if the module in node_module was installed from that branch by reading _from
• a tag in package.json is always valid with node_modules because the tag could have pointed at anything
• a tag in package.json is valid with node_modules, if the module in node_modules was installed from that tag by reading _from
• a tag in package.json is it's in the form of #v{semver} is valid if the module in node_modules is at version {semver}. This makes assumptions about auto generated tags from npm version {semver}.
• a tag or branch in package.json is valid with node_modules, if after we have asked the git remote what commit the tag / branch resolves to matches the resolved commit in node_modules. This option is expensive as we have to query the git remote.

The problem is fundamentally the notion that node_modules is stale if you change the version of the codebase. This problem also exists when you change a branch.

the hard part to do as an external module is hooking into npm run anything. We would need a pre run script or wrap npm with a wrapper command and alias npm='better-npm'.

Also with the default --save from a local npmrc and with it mutating npm-shrinkwrap.json we can start assuming that node_modules is NOT the source of truth and have any staleness in node_modules be automatically resolved with an npm install from shrinkwrap or package.json.

Questions:

• how do we implement this outside of npm core. What mechanism do we have to hook into "before any operation".
• should we make this git aware and how so ?
• should we update npm ls to be git aware and how so ?

[othiym23]

Reading the installed tree from disk isn’t a cheap operation; particularly with the large trees generated by ember-cli, it’s a very memory- and CPU-intensive operation. Given that performance is already a major concern for what feels like a majority of CLI users, this narrows the applicability of this kind of check to the relatively small subset of operations that directly deal with installed dependencies (particularly npm install and npm publish).

Re: @Raynos’s points about shrinkwrap behavior when checking out branches, npm@3 already handles many of the cases he describes. When changing branches, npm install will now bring the installed tree into line with what’s in the changed shrinkwrap file. That doesn’t cover all the use cases described by @domenic and @stefanpenner, but it does make keeping your tree in sync much less painful for most users most of the time.

The team agrees that in general it would be useful to help users not push repos or publish packages with package trees in inconsistent states. As a few of you have alluded, this is something that could be built out as a separate utility, and added to projects via package lifecycle scripts or repo hooks (which also goes a long way towards remedying the performance concerns above – if you’re consciously opting into this functionality, you’re much less likely to be displeased with the resulting slowdown). I think that’s the best way to lay down a cowpath that the CLI could later pave by incorporating this functionality into npm ~lint (final name TBD) that could be tied to the installation or publishing workflow. There’s a chance that the CLI team itself will get to doing this itself, as part of improving the user experience of locking dependencies with the CLI (a project we’re still in only the earliest stages of defining). I can’t commit to that, though, and so I’m going to close this issue. If you put together something to do this, please bring it to our attention.

We further agree that when using the CLI to manage dependencies, it should never be possible to put package.json and npm-shrinkwrap.json at odds. In the cases where that (still) happens, those are bugs that should be fixed, so please file them as such.

Thanks to all for the discussion, as well as for your time and patience!