Skip to content

Add option to control whether setup-node is used for cache fallback#67

Closed
mcmire wants to merge 1 commit into
mainfrom
dont-fallback-to-setup-node
Closed

Add option to control whether setup-node is used for cache fallback#67
mcmire wants to merge 1 commit into
mainfrom
dont-fallback-to-setup-node

Conversation

@mcmire

@mcmire mcmire commented Apr 28, 2026

Copy link
Copy Markdown
Contributor

Description

Currently, this action uses two strategies for caching node_modules. It will first attempt to restore a previous cache, and if there is a cache miss, the action will fall back to actions/setup-node and its cache.

Here's we've observed in other repos such as the extension: if yarn.lock is updated in a PR, the first job to run this action will correctly see a cache miss (since the cache key is based on the contents of yarn.lock). However, actions/setup-node may report a cache hit. That is very strange, because its cache key is supposedly also based on the contents of yarn.lock — except even stranger, the hash of yarn.lock that setup-node computes is different from the one that this action computes.

We haven't been able to nail down the exact differences between this action and actions/setup-node that would cause this behavior. Regardless, it may not be necessary to fall back to actions/setup-node for cache restoration. Therefore, this commit adds an option so that the fallback can be disabled.

References

See this Slack thread for more information: https://consensys.slack.com/archives/C094GV3E7SB/p1777056238889789


Note

Low Risk
Low risk: only adds a new optional input that gates an existing cache-restore step, changing behavior only when explicitly disabled.

Overview
Adds a new use-setup-node-for-cache-fallback input (default true) to control whether the action falls back to actions/setup-node’s Yarn cache restore when the node_modules cache is missed.

When set to false, the Conditionally restore action/setup-node cache step is skipped, preventing setup-node from restoring .yarn as a secondary cache source.

Reviewed by Cursor Bugbot for commit 7ddb11a. Bugbot is set up for automated code reviews on this repo. Configure here.

Currently, this action uses two strategies for caching `node_modules`.
It will first attempt to restore a previous cache, and if there is a
cache miss, the action will fall back to `actions/setup-node` and its
cache.

Here's we've observed in other repos such as the extension: if
`yarn.lock` is updated in a PR, the first job to run this action will
correctly see a cache miss (since the cache key is based on the contents
of `yarn.lock`). However, `actions/setup-node` may report a cache *hit*.
That is very strange, because its cache key is supposedly _also_ based
on the contents of `yarn.lock` — except even stranger, the hash of
`yarn.lock` that `setup-node` computes is different from the one that
this action computes.

We haven't been able to nail down the exact differences between this
action and `actions/setup-node` that would cause this behavior.
Regardless, it may not be necessary to fall back to `actions/setup-node`
for cache restoration. Therefore, this commit adds an option so that the
fallback can be disabled.
@mcmire mcmire marked this pull request as ready for review April 28, 2026 21:42
@mcmire mcmire requested a review from a team as a code owner April 28, 2026 21:42
@Gudahtt

Gudahtt commented Apr 28, 2026

Copy link
Copy Markdown
Member

The setup-node cache is for the Yarn cache, which still requires running yarn install. Whereas when we cache node_modules, we don't have to run yarn install. They're not really equivalent.

It's not clear to me why we'd ever want to disable the setup-node cache in this case

@mcmire

mcmire commented Apr 29, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of #66.

@mcmire mcmire closed this Apr 29, 2026
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