perf(cli): cache the Node install layer across source-only changes - #8607
Open
Elior Nataf Lackritz (eliornl) wants to merge 2 commits into
Open
perf(cli): cache the Node install layer across source-only changes#8607Elior Nataf Lackritz (eliornl) wants to merge 2 commits into
Elior Nataf Lackritz (eliornl) wants to merge 2 commits into
Conversation
The generated Node Dockerfile copies source before running the install, so the first source change invalidates the install layer and every dependency is reinstalled. A production build showed 0/3 steps cached with `npm ci` rebuilding at 16.9s. Copy `package.json` and the lockfile first instead. Falls back to the current output when there is no lockfile, when an install lifecycle hook is present, when a custom install command is set, or when the config is nested in a workspace.
![open-swe[bot]](https://avatars.githubusercontent.com/in/1395372?s=60&v=4){kind=small}
npm runs prepublish, preprepare and postprepare on install too, so a project using one of those would have had its install layer split and the hook would run against a source tree that is not there yet.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The generated Node Dockerfile copies source before running the install, so the first source change invalidates the install layer and every dependency is reinstalled. A production build showed
0 / 3steps cached, withnpm cirebuilding at 16.9s.Copies
package.jsonand the lockfile first instead.Falls back to the current output in four cases: no lockfile, an install lifecycle hook present, a custom install command, or a config nested in a workspace. A lockfile is required because without one the install resolves versions at build time, so caching that layer could pin an older resolution than a clean build.
Red/green: 2 of the 10 tests fail on main, the ones asserting the new ordering. The other 8 guard the fallbacks and pass on main by design, so a later change cannot silently widen the optimization. Confirmed the nested-config guard has teeth by deleting it: that test fails and nothing else does.
uv_lock.pychanges by one character because_get_node_pm_install_cmdnow returns the lockfile alongside the command. Kept as one function since the two facts come from the same directory scan; splitting them means two functions that must agree on package-manager priority.