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

Use actions/setup-node to cache pnpm store instead of doing it manually #76

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dtinth
Copy link

@dtinth dtinth commented Mar 14, 2023

actions/setup-node supports the option cache: pnpm, so we can take advantage of that to make our workflow simpler.

image

Note: pnpm/action-setup must be run BEFORE actions/setup-node because otherwise the setup-node action will fail due to not being able to find the pnpm executable.

Copy link

@menasheh menasheh left a comment

Choose a reason for hiding this comment

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

Haven't tried the old way for comparison, but can confirm this way works.

@robotkutya
Copy link

Anyone knows what the difference is between the two methods?
#82

@nicolassanmar
Copy link

nicolassanmar commented Jun 12, 2023

I don't know if I have set this up incorrectly, but I find the manual way to be faster than using the cache from setup-node.

The manual way does not require us to install the packages again (as we cache node_modules), while this suggested one does require it:

Manual (~20s)

image

Using setup-node (~1m)

(This was a cache hit)
image

setup-node step config

    - name: Install pnpm
      uses: pnpm/action-setup@v2
      with:
        version: 8.1.0
        run_install: false

    # Has to be done after installing pnpm, otherwise the cache will not work.
    - name: Install Node.js
      uses: actions/setup-node@v3
      with:
        node-version: 18
        cache: pnpm
        cache-dependency-path: 'pnpm-lock.yaml'

@dtinth
Copy link
Author

dtinth commented Jun 12, 2023

@nicolassanmar Does your “manual” setup also cache node_modules? Using cache: pnpm merely caches the pnpm store, not node_modules.

This is how I interpret the log message:

Log message Description
resolved X, reused 0, downloaded X, added X, done Cache is not working.
resolved X, reused X, downloaded 0, added X, done Store cache is working.
Already up to date node_modules is cached.

@nicolassanmar
Copy link

nicolassanmar commented Jun 12, 2023

@dtinth You are 100% right. I thought that https://github.com/pnpm/action-setup#use-cache-to-reduce-installation-time suggested caching node_modules but I was mistaken.
I am manually caching node_modules and I don't think setup-node provides a way to do that
They stated that it is not supported and they are not planning to add it actions/setup-node#406 (comment)

@robotkutya
Copy link

thanks guys for the discussion, makes it much more clear!

I think more info about this would be appreciated to newcomers, any ideas how to improve the docs in this regard?

@FFdhorkin
Copy link

FFdhorkin commented Sep 27, 2023

I would highly suggest you show both as two different options (as opposed to replacing the existing one) - this solution won't work if you're using a self-hosted runner that has node pre-installed

@davidlj95
Copy link

pnpm docs also mention using setup-node to cache pnpm store

https://pnpm.io/continuous-integration#github-actions

I think we should update the example to go the recommended way by using setup-node. I think that would be the recommended way given you don't have to maintain a custom cache. setup-node takes care of that for you (what if pnpm store command is deprecated and now another one is used? setup-node would take care of that). So a thing less to worry about

Links:

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.

6 participants