Skip to content

Improve Node.js caching using package-lock.json - #409

Merged
ppkarwasz merged 4 commits into
mainfrom
fix/revert-npm-shrinkwrap
Jun 10, 2025
Merged

Improve Node.js caching using package-lock.json#409
ppkarwasz merged 4 commits into
mainfrom
fix/revert-npm-shrinkwrap

Conversation

@ppkarwasz

Copy link
Copy Markdown
Member

This PR updates the caching strategy for Node.js dependencies to base the cache key on the contents of package-lock.json, rather than the node or node_modules directories.

Problem

Currently, caching is ineffective because:

  • The cache key relies on node and node_modules directories.
  • These directories are not checked into the repository and therefore don’t exist when the cache is computed.

Solution

Update the cache key to use the contents of package-lock.json, which is a reliable representation of the dependency tree:

  • If package-lock.jsonis not committed, the cache behavior remains unchanged.
  • If it is committed, caching will function as expected, restoring and saving dependencies based on lockfile changes.

Additional Changes

  • Commit package-lock.json in the logging-parent repository to evaluate the effectiveness of this approach.
  • Reverts the use of npm-shrinkwrap.json introduced in Lock Antora dependencies #367.

This PR updates the caching strategy for Node.js dependencies to base the cache key on the contents of `package-lock.json`, rather than the `node` or `node_modules` directories.
### Problem
Currently, caching is ineffective because:
* The cache key relies on `node` and `node_modules` directories.
* These directories are not checked into the repository and therefore don’t exist when the cache is computed.
### Solution
Update the cache key to use the contents of `package-lock.json`, which is a reliable representation of the dependency tree:
* If `package-lock.json` **is not committed**, the cache behavior remains unchanged.
* If it **is committed**, caching will function as expected, restoring and saving dependencies based on lockfile changes.
### Additional Changes
* Commit `package-lock.json` in the `logging-parent` repository to evaluate the effectiveness of this approach.
* Reverts the use of `npm-shrinkwrap.json` introduced in #367.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the Node.js caching strategy by basing the cache key on the contents of package-lock.json instead of transient directories. Key changes include:

  • Updating the changelog to reflect the new caching approach.
  • Adjusting pom.xml by replacing version properties and removing an outdated npm install configuration.
  • Revising the GitHub Actions workflow to restore the cache using the hash of package-lock.json and adding a step to save the Node.js cache.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

FileDescription
src/changelog/.12.x.x/improve-nodejs-cache.xmlUpdated changelog with a new issue id and description to document the caching improvement.
pom.xmlModified version properties and removed deprecated npm install configuration.
.github/workflows/deploy-site-reusable.yamlUpdated caching steps: replacing the old key with one based on package-lock.json and adding explicit restore/save steps.

Comment thread.github/workflows/deploy-site-reusable.yaml Outdated
vy
vy approved these changes Jun 10, 2025
@ppkarwasz
ppkarwasz merged commit e6f8ac3 into mainJun 10, 2025
@ppkarwasz
ppkarwasz deleted the fix/revert-npm-shrinkwrap branch June 10, 2025 09:38
ppkarwasz added a commit that referenced this pull request Jun 10, 2025
The `key` parameter for the `actions/cache/save` action introduced in #409 is not evaluated due to missing expression syntax.
ppkarwasz added a commit that referenced this pull request Jun 10, 2025
The `key` parameter for the `actions/cache/save` action introduced in #409 is not evaluated due to missing expression syntax.
Sign up for freeto 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.

3 participants

@ppkarwasz@vy