feat(builtin): use npm ci as default behaviour for installing node_modules #2328
Conversation
lukasholzer
requested review from
alexeagle,
gregmagolan,
mattem and
soldair
as code owners
lukasholzer
force-pushed
the
feat/npm-ci-default
branch
from
bb0989e
to
cddd4fe
Compare
lukasholzer
force-pushed
the
feat/npm-ci-default
branch
2 times, most recently
from
dfaaa69
to
ee5b9df
Compare
internal/npm_install/npm_install.bzl
Outdated
| @@ -303,6 +312,18 @@ npm_install = repository_rule( | |||
| See npm CLI docs https://docs.npmjs.com/cli/install.html for complete list of supported arguments.""", | |||
| default = [], | |||
| ), | |||
| "npm_ci": attr.bool( | |||
There was a problem hiding this comment.
a boolean is a strange way to model a string.
what should a user expect that npm_ci = False means? I don't think it would be very clear that it means "use npm install instead"
maybe it should be
"npm_command": attr.string(default = "ci", doc = "which npm command to run to install dependencies", values = ["ci", "install"])
There was a problem hiding this comment.
A very good idea. I was not 100% happy with the naming but I don't have something better in mind. Your suggestion is a perfect solution, will adapt the changes :)
lukasholzer
force-pushed
the
feat/npm-ci-default
branch
from
ee5b9df
to
1d187ff
Compare
lukasholzer
force-pushed
the
feat/npm-ci-default
branch
from
1d187ff
to
0ec59a0
Compare
…dules To be more hermetic with the install of the dependencies use npm ci to install the exact version from the package-lock.json file. To update a dependency use the vendored npm binary with `bazel run @nodejs//:npm install <dep-name>`. Fixes bazel-contrib#159
lukasholzer
force-pushed
the
feat/npm-ci-default
branch
from
0ec59a0
to
887c496
Compare
|
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
|
@googlebot I consent. |
To be more hermetic with the install of the dependencies use npm ci to install the exact version from the package-lock.json file.
To update a dependency use the vendored npm binary with
bazel run @nodejs//:npm install <dep-name>.Fixes #159
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Currently, the npm_install rule will use the npm install command and installs the latest version that is possible in the range of the packag.json
Issue Number: #2327 159
What is the new behavior?
The new behaviour is using the
npm cicommand to install the dependencies, this means that it will install the exact version from the package-lock.json fileDoes this PR introduce a breaking change?
Other information