s2i-nodejs-container
s2i-nodejs-container copied to clipboard
sclorg
Adding ability to run npm ci by default
Making npm ci to run by default and if it fails for any reason we will run npm install as a backup, this will prevent build from failure for developers who doesn't use or update package-lock.json.
This is the one way and another way is to use variable RUN_NPM_CI like we use NODE_ENV, but that would require existing developers to update their build_template and for new developers who are not aware of this env variable won't be able to use this approach.
Please refer below docs for more information: npm ci npm install
#212 @phracek please review.
@phracek that is correct will push updated PR. Also is there any changes that I have to make for the test to be successful?
@aksgupta14 The failure is:
---> Installing dependencies with npm ci
[91mnpm ERR! cipm can only install packages with an existing package-lock.json or npm-shrinkwrap.json with lockfileVersion >= 1. Run an install with npm@5 or later to generate it, then try again.
[0m[91m
npm ERR! A complete log of this run can be found in:
npm ERR! /opt/app-root/src/.npm/_logs/2020-04-27T08_37_33_712Z-debug.log
[0m---> npm ci failed, installing dependencies with npm install
@phracek after going through the test cases, I was able to figure out the issue. The issue appears only with incremental build and the reason being npm ci removes all packages inside node_modules even though it fails, now when npm install is run it installs all the packages that is 121 packages which make it equal to the previous build, hence incremental build errors out by giving error ERROR Incremental build failed: both builds installed 121 packages.
Now when we will use npm ci it will always install the all packages irrespective of the packages already installed which will make this test case irrelevant. can you please advice as to how I should proceed with updating test cases?
I am also looking for that capability, is there a plan to make npm ci avalable in the S2I?
This PR seems to make the change for Node.js versions 10 and 12 which are no longer supported. While I can see the change with a working fallback makes sense to me I'd suggest it should only be made for the currently supported LTS versions.
This is probably because it was submitted many years ago. At this point I think we should likely only do for 16 so that we minimize risk to people already using the containers.
hi folks, friendly ping here about this PR.
We found a situation that maybe this feature can help us: https://github.com/nodeshift-starters/devfile-sample/issues/10#issue-1318826158
This error is not happening when using the version 14.
@aksgupta14 are you still willing/interested in getting this feature added? I know this is quite old but looking to see how we move it forward.
@aksgupta14 Can you please update this pull request? There are several conflicts.