-
Notifications
You must be signed in to change notification settings - Fork 23
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
[Housekeeping] Cache Node Modules #1494
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
MadsBuchmann
changed the title
Housekeeping/1360 cache node modules
[Housekeeping] Cache Node Modules
Apr 28, 2021
jakobe
requested changes
May 3, 2021
jakobe
approved these changes
May 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR closes #1360
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Which issue documents the current behaviour?
Issue Number: #1360
What is the new behavior?
Before installing dependencies it is attempted to fetch a cached version of
node_modules
using a cache-key. If there is a cache-hit, the cachednode_modules
are used andnpm run postinstall
is executed. In the case of a cache miss, a clean install is executed by runningnpm ci
- the resultingnode_modules
is then cached.The cache-key contains:
package.json
package-lock.json
The OS and Node version such that we only retrieve caches created in the same environment, and the hash of package.json such that we only retrieve caches created with the same dependencies, post-install scripts etc.
Why use a mix of
npm ci
andnpm run postinstall
It seems common consensus across the cool tech blogs is to not cache
node_modules
asnpm ci
does a clean install and therefore deletes this folder as part of the process.Instead you should:
~/.npm
which is the cache NPM uses to speed up package installation.npm ci
to create a re-creatable deploy frompackage-lock.json
which will be sped up due to the cached~/.npm
I started out by doing that and it did speed up the install step a tiny bit (like 20 seconds or something). But the majority of the time spent during the install step, actually came from our post-installation scripts which among other things runs ngcc. The result of ngcc is stored in
node_modules
.So I opted for a sort of mixed strategy:
npm ci
) which runs post-install scripts and cache the results. This is a strict install as no changes to package-lock.json should occur (npm ci
fails ifpackage.json
andpackage-lock.json
is out of sync).node_modules
and just runnpm run postinstall
! We can do this safely, because we know the cache originates from a strict install, that used the samepackage.json
andpackage-lock.json
! We need to run postinstall as it runs some transpilation among other things that outputs to other folders thannode_modules
.Pipeline when there is a cache-hit:
Pipeline when there is a cache-miss:
In general the install steps has gone from taking approx 2 minutes to 22 seconds, as long as there is a cache :)
Does this PR introduce a breaking change?
Other information
I've created two issues for additional improvement of our CI pipeline: