-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Enable caching by default if packageManager
field is defined in package.json
#686
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
Comments
I think the implementation can be as simple as - if (cache && isCacheFeatureAvailable()) {
+ const packageManagerFromManifest = getNameFromPackageManagerField();
+ if ((cache !== "" || packageManagerFromManifest) && isCacheFeatureAvailable()) {
const cacheDependencyPath = core.getInput('cache-dependency-path');
- await restoreCache(cache, cacheDependencyPath);
+ const packageManager = cache !== "" ? cache : packageManagerFromManifest;
+ await restoreCache(packageManager, cacheDependencyPath);
} where
Other package managers can be added, once they are supported |
Hello @trivikr ! Thank you for the suggested idea! |
This is what I doing before this feature landed: jobs:
steps:
- name: Checkout
uses: actions/checkout@v3
- name: Detect package manager
shell: bash
run: node -p "'PACKAGE_MANAGER='+require('./package.json').packageManager?.split?.('@')?.[0]" >> "$GITHUB_ENV"
- name: Install pnpm
run: corepack enable pnpm
if: ${{ env.PACKAGE_MANAGER == 'pnpm' }}
# install pnpm before setup-node
- name: Setup node
uses: actions/setup-node@v3
with:
node-version: lts
cache: ${{ env.PACKAGE_MANAGER }} |
Revisiting this, as devEngines has now appeared as a standard in Node.js ecosystem. It's implemented by npm@10.9.0 as well as corepack@0.32.0. |
Description:
The action allows caching global packages data as per documentation in https://github.com/actions/setup-node#caching-global-packages-data
It's an amazing feature and reduces action execution time, but it has to be enabled in configuration.
Caching is not enabled in default setup. Users may not discover caching feature by default, and miss on the faster executions times enabled by caching.
The feature request is to enable caching by default if
packageManager
field is defined.This can be enabled in a major version bump, say
v4
, to not affect existing users.Justification:
Just want all users to get benefit of faster executions times allowed by caching without explicitly opting in, if they have set up packageManager field in their package.json
They can always explicitly opt-out by setting
cache
to empty string.Are you willing to submit a PR?
Yes
The text was updated successfully, but these errors were encountered: