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

Add pnpm, yarn & bun support #42

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

andershagbard
Copy link
Contributor

@andershagbard andershagbard commented Apr 29, 2024

This PR makes the action use the package manager defined by the repository. If the package isn't available for the runner, it will fall back to npm.

@charlespwd comment

  • figure out why pnpm doesn't like --no-lockfile (or figure out the proper flag that lets you install without lockfile changes)

Not sure if this was incorrect. Does this action output errors in the job log?

  • consider making a global install (?)

Kept it as local install in this PR.

Tested with

  • pnpm
  • npm
  • yarn
  • bun

Resolves #37

"common-tags": "^1.8.2",
"detect-package-manager": "^3.0.1",
"semver": "^7.5.4",
"which": "^4.0.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure this works on Windows runners? Didn't see a comment about it in the README

Copy link
Contributor

Choose a reason for hiding this comment

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

@charlespwd
Copy link
Contributor

Not sure if this was incorrect. Does this action output errors in the job log?

I didn't test this, I only saw that in the logs from the report and it looked sus. Haven't played with pnpm yet, never felt the need to upgrade tbh (and every time I see a pnpm related issue, it feels like a waste of time 😅. I see a whole lot more of those than yarn.)

image

@andershagbard
Copy link
Contributor Author

I don't think that WARN is an error. Seems to work fine with pnpm.

@andershagbard andershagbard marked this pull request as ready for review May 8, 2024 12:22
@andershagbard
Copy link
Contributor Author

@charlespwd What is the status on this?

Running npm install @shopify/cli after you already run pnpm install is very time consuming.

This is my action with pnpm install:
image

This is my action without running pnpm install:
image

Notice Theme Check is 4x slower.

I do this because I am compiling JS and CSS. Theme Check will return errors for MissingAsset if I don't.

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.

Give support to pnpm workspace
2 participants