Skip to content

ci: include package manifest in dependency cache keys - #1175

Merged
rfgamaral merged 1 commit into
mainfrom
ricardo/include-package-manifest-in-cache-key
Aug 18, 2026
Merged

ci: include package manifest in dependency cache keys#1175
rfgamaral merged 1 commit into
mainfrom
ricardo/include-package-manifest-in-cache-key

Conversation

@rfgamaral

Copy link
Copy Markdown
Member

🗺 Overview

A recent Reactist dependency update introduced a peer dependency conflict. Because the update changed package.json without changing package-lock.json, CI restored cached node_modules, skipped npm ci, and passed with the previous dependency installed. A later lockfile change invalidated the cache and exposed the installation failure, blocking unrelated work.

After fixing the immediate Reactist issue, I audited other active projects for the same cache pattern. This PR adds package.json to the node_modules cache key so manifest-only changes run dependency installation and expose failures immediately.

This can cause an extra cache miss when a package.json change does not affect installed dependencies, but it’s a small price to pay for catching broken dependency changes before they merge and block unrelated work.

🔗 Reference

@doistbot doistbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR adds package.json to the node_modules cache keys across all workflow files so that manifest-only dependency changes invalidate the cache and force npm ci to run, catching conflicts before they merge. No inline issues were flagged.

I also left one optional follow-up note in the details below.

Optional follow-up note (1)
  • P3 .github/workflows/check-ci-validation.yml:41: package.json is a bare path while the other entries in this hashFiles call are globs (**/package-lock.json, **/.node-version). Today there's only a root package.json, so it's functionally fine, but if a sub-package is ever added its package.json changes wouldn't invalidate the cache while its package-lock.json would — the exact manifest/lockfile asymmetry this PR is meant to prevent. Using **/package.json keeps it consistent with the surrounding globs.

Share FeedbackReview Logs

@rfgamaral rfgamaral self-assigned this Aug 18, 2026
@rfgamaral rfgamaral added the 👀 Show PR Used for PRs that need a review, but can be merged when CI is green. label Aug 18, 2026
@rfgamaral
rfgamaral requested review from a team and nvignola and removed request for a team August 18, 2026 13:51
@rfgamaral
rfgamaral merged commit 1ad0b40 into main Aug 18, 2026
10 of 12 checks passed
@rfgamaral
rfgamaral deleted the ricardo/include-package-manifest-in-cache-key branch August 18, 2026 13:51

@nvignola nvignola left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM ✅

@doist-release-bot

Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 37.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@doist-release-bot doist-release-bot Bot added the Released PRs that have been merged and released label Aug 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Released PRs that have been merged and released 👀 Show PR Used for PRs that need a review, but can be merged when CI is green.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants