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

Adopt pnpm as the default package manager #870

Merged
merged 15 commits into from
Dec 7, 2022
Merged

Adopt pnpm as the default package manager #870

merged 15 commits into from
Dec 7, 2022

Conversation

pepicrft
Copy link
Contributor

@pepicrft pepicrft commented Dec 6, 2022

WHY are these changes introduced?

Despite Yarn having done a reasonably good job at managing the dependencies of the project, there are a few issues rooted in their design that might manifest on the user side and worse the experience contributing to the repo. In particular:

  • Yarn's hoisting mechanism combined with how the Node module system works might lead to packages depending on dependencies that are not declared as their dependencies. This can go unnoticed when building and testing the packages locally and on CI. Some projects prefer to use ESLint to catch these issues.
  • Yarn's management of the global cache and the IO operations (copying and pasting files from the cache into node_modules) proved to be inconsistently unreliable leading to yarn install failures that cause developers to either retry a build on CI or deleting node_modules locally and trying again.

WHAT is this pull request doing?

I'm replacing Yarn with PNPM.

PNPM addresses the above issues first, by being more strict with the directory structure and the usage of peerDependencies, and second, by having a global cached that's accessed through symlinks. PNPM's has a more sophisticated caching system with a content-addressable store that leads to faster dependency installation.

Even though the above issues might seem minor, they'll become more frequent as our monorepo grows. I'm confident this is a great step forward for contributors and users. Other projects like Vite or Vitest have already validated the tool and use it at a much larger scale than us.

Why not Yarn 2/3?

They made plug & play the default approach to expose the module graph to the runtime and it's not compatible with ESM yet. The API they'd need to build upon, loaders, it's flag as experimental in Node. Moreover that approach is not conventional and therefore requires additional setup of our transpilation tools and workflows to make them work. This will make the project harder to reason about.

How to test your changes?

  1. Check out the branch
  2. Run rm -rf node_modules/ **/node_modules & pnpm install
  3. Run pnpm build && pnpm lint && pnpm test
  4. yarn should output a message telling you to use pnpm instead.

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
  • Existing analytics will cater for this addition
  • PR includes analytics changes to measure impact

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

@pepicrft pepicrft self-assigned this Dec 6, 2022
@pepicrft pepicrft requested review from Arkham, shauns and a team December 6, 2022 17:10
@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

  • Themes: @shopify/theme-developer-tools
  • UI extensions: @shopify/ui-extensions-cli
  • Hydrogen: @shopify/hydrogen
  • Other: @shopify/cli-foundations

@pepicrft pepicrft requested a review from a team December 6, 2022 17:53
@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run "yarn changeset add" to track your changes and include them in the next release CHANGELOG.

Copy link
Contributor

@Poitrin Poitrin left a comment

Choose a reason for hiding this comment

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

🎩 passed ✅ and code LGTM!

Reasons to move to pnpm make sense, even though we then have yet another package manager :D

I just have some questions left :)

bin/shadowenv/yarn Outdated Show resolved Hide resolved
fixtures/app/web/package.json Show resolved Hide resolved
packages/cli-hydrogen/package.json Show resolved Hide resolved
packages/features/cucumber.js Show resolved Hide resolved
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