srs / gradle-node-plugin

Gradle plugin for integrating NodeJS in your build. :rocket:
Apache License 2.0
866 stars 211 forks source link

npm install should be using npm ci #307

Open tobias- opened 6 years ago

tobias- commented 6 years ago

As of NPM 5.7.0 there's a new command for npm called ci which uses the package-lock.json to decide which dependencies to install. A normal npm install will just overwrite package-lock.json with the latest acceptable versions of the dependencies (and their dependencies) and tell the user that X packages has been updated. This is ok-ish for promoted builds, but works poorly where you do a specific production build and expect dependencies to be the same unless someone explicitly updates the dependency-"manifest".

I believe that changing the default behaviour from install to ci would be better in the long run, but it's a major change, creating a flag that toggles between install and ci that defaults to the old behaviour would probably be preferable. Especially since the flag didn't exist until 5.7.0.

bsautel commented 5 years ago

As a workaround, I created manually the npmCi task in the build script:

task npmCi(type: NpmTask) {
    dependsOn npmSetup
    npmCommand = ["ci"]
    inputs.file("package.json")
    inputs.file("package-lock.json")
    outputs.dir("node_modules")
}
web-devel commented 5 years ago

The built-in ci task would be great but implicitly substitute install with ci is wrong, at least it will break projects without package-lock.json.

A normal npm install will just overwrite package-lock.json with the latest acceptable versions of the dependencies (and their dependencies) and tell the user that X packages has been updated.

It's not true.

deepy commented 5 years ago

I have a proposed solution in a fork. https://github.com/node-gradle/gradle-node-plugin/pull/5

If anyone would like to have a look at it that'd be great!

The idea is to add a task that changes the command of npmInstall. In my case I used npmCi

rafeememon commented 5 years ago

Perhaps a less intrusive solution would be to add a configuration option to node { ... } that would toggle the behavior of the npmInstall task. This could be toggled pretty easily when running in CI. For example, for CircleCI:

node {
  ...
  ci = System.env.CI
}
danielsuter commented 5 years ago

So is it possible now to use npm ci? If I add another task, implicitely npmInstall will still be triggered, correct?

bsautel commented 5 years ago

Yes it is possible. And no, the npm install task is not triggered automatically, at least when you use custom tasks extending NodeTask.