Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch from "npm ci" to "npm install" for CI #719

Merged
merged 3 commits into from
Nov 3, 2021
Merged

Conversation

aduth
Copy link
Member

@aduth aduth commented Nov 3, 2021

Why: Because "npm ci" ignores and destroys the cached node_modules and is otherwise difficult to cache. "npm install" should behave similar, respect package-lock.json, hydrate from cache. The other behavior of "npm ci" of validating package-lock.json isn't always reliable, and is replicated here with check-lockfiles, largely copied from similar scripts in identity-style-guide and identity-idp.

Related: npm/cli#564

From example build:

Found a cache from build 8619 at v2-dependencies-gGKVNxLRg3iHjN88fQag16qWfIEt44dSLDpRuOkhS2s=
Size: 170 MiB
Cached paths:
  * /home/circleci/project/node_modules

Downloading cache archive...
Validating cache...

Unarchiving cache...

# ...

npm WARN prepare removing existing node_modules/ before installation

**Why**: Because "npm ci" ignores and destroys the cached node_modules and is otherwise difficult to cache. "npm install" should behave similar, respect package-lock.json, hydrate from cache. The other behavior of "npm ci" of validating package-lock.json isn't always reliable, and is replicated here with check-lockfiles, largely copied from similar scripts in identity-style-guide and identity-idp.
Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

.circleci/config.yml Outdated Show resolved Hide resolved
aduth and others added 2 commits November 3, 2021 14:04
**Why**: Was previously always skipping
@aduth
Copy link
Member Author

aduth commented Nov 3, 2021

The overall time doesn't seem to have been affected since the E2E tests take a long time, but there's a pretty noticeable improvement for all other tasks:

main aduth-ci-npm-install
main times branch times

@aduth aduth merged commit 7a5ef1b into main Nov 3, 2021
@aduth aduth deleted the aduth-ci-npm-install branch November 3, 2021 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants