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

ci: Stabilize CI dependency cache key #13401

Merged
merged 3 commits into from
Aug 27, 2024
Merged

ci: Stabilize CI dependency cache key #13401

merged 3 commits into from
Aug 27, 2024

Conversation

mydea
Copy link
Member

@mydea mydea commented Aug 16, 2024

Ensure it only changes if actual dependencies change.

Previously, we would invalidate the dependency cache every time a package.json of the workspace changed in any way. This is defensive, but it also means that we also invalidate if one of these things happen:

  1. A script or similar is added/edited for workspace package
  2. A release is made, bumping internal dependency versions

This change updates this to calculate the hash with a slightly more sophisticated approach, which should hopefully ensure we only actually bust the cache when a dependency actually changes. This should lead to the dependency cache being re-used much more, because only rarely is an actual dependency changed.

Ensure it only changes if actual dependencies change.
@mydea mydea self-assigned this Aug 16, 2024
@mydea mydea changed the title ci: Stabilize CI cache key ci: Stabilize CI dependency cache key Aug 16, 2024
.sort()
.forEach(key => {
// If the dependency is a workspace package, ignore the version
// No need to invalidate a cache after every release
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this mean that we might have outdated internal code in the cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is the cache key to cache the node_modules folder. So all the workspace packages should not be there at all, so the version should not matter (I think? at least 😅 )

Copy link
Member

Choose a reason for hiding this comment

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

👍 yeah I just wasn't sure how these workspace packages are handled within node modules

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, it's a good question 🤔 there are only some symlinks in there, but these should not change I believe between versions. IMHO we can try it and if there are problems we can revisit this particular check - but I'd expect it to be OK. We do capture added internal dependencies, which should be fine I think/hope 😅

@mydea mydea marked this pull request as ready for review August 16, 2024 09:33
@mydea mydea merged commit 195482c into develop Aug 27, 2024
128 checks passed
@mydea mydea deleted the fn/ci-dep-cache-key branch August 27, 2024 10:37
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