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

Upgrade to Yarn 3 (attempt 3) #2841

Merged
merged 8 commits into from Aug 2, 2022
Merged

Conversation

srmagura
Copy link
Member

@srmagura srmagura commented Jul 31, 2022

Previous attempt at this: #2657

Benefits

  • Yarn 1 is virtually unmaintained so it's good to move off of it.
  • Anecdotally, Yarn 3 is faster than Yarn 1, especially when doing an install after switching branches.
  • We are still using node_modules, not Plug'n'Play, so this upgrade didn't break anything in the repository.

Misc Notes

  • I removed the jest-snapshot package patch because I don't think patch-package is compatible with Yarn 3. Removing the patch doesn't seem to have caused any problems though??
  • I fixed the ssr-benchmarks project — it had an import which stopped working when we added the exports field to package.json, I think.
  • Removed opencollective postinstall from postinstall script since the output does not actually get printed to stdout when using Yarn 3.

@srmagura srmagura requested a review from Andarist July 31, 2022 16:08
@changeset-bot
Copy link

changeset-bot bot commented Jul 31, 2022

⚠️ No Changeset found

Latest commit: a7993c2

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 31, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit a7993c2:

Sandbox Source
Emotion Configuration

@srmagura srmagura marked this pull request as ready for review July 31, 2022 16:34
@Andarist
Copy link
Member

Andarist commented Aug 2, 2022

I removed the jest-snapshot package patch because I don't think patch-package is compatible with Yarn 3. Removing the patch doesn't seem to have caused any problems though??

I think that it should be compatible with node_modules linker but I might be mistaken. You could try to use yarn patch to replace this: https://yarnpkg.com/cli/patch

The patch is needed for updating inline snapshots and I don't think that the issue has been solved in Jest yet so we might need to bring this patch back.

- [Node.js](http://nodejs.org/) >= v7 must be installed.
- [Yarn](https://yarnpkg.com/en/docs/install)
- [Node.js](http://nodejs.org/) >= v16 must be installed.
- We are using Yarn Modern so you need either a global install of Yarn v1 (`npm i -g yarn`) or you need to [enable Corepack](https://yarnpkg.com/getting-started/install).
Copy link
Member

Choose a reason for hiding this comment

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

I never have seen it being called "Yarn Modern", I think they usually refer to it as "Yarn Berry":

Suggested change
- We are using Yarn Modern so you need either a global install of Yarn v1 (`npm i -g yarn`) or you need to [enable Corepack](https://yarnpkg.com/getting-started/install).
- We are using Yarn Berry so you need either a global install of Yarn v1 (`npm i -g yarn`) or you need to [enable Corepack](https://yarnpkg.com/getting-started/install).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think "Berry" is something of a codename. See here where they call it Yarn Modern.

@srmagura
Copy link
Member Author

srmagura commented Aug 2, 2022

Hmm, yarn test -u is correctly updating inline snapshots for me regardless of whether I have the patch. I tested by changing a color value in packages/cache/__tests__/hydration.js.

Anyway, I added the patch using the native Yarn 3 patching feature. Let me know if it works for you.

@Andarist
Copy link
Member

Andarist commented Aug 2, 2022

Hmm, yarn test -u is correctly updating inline snapshots for me regardless of whether I have the patch. I tested by changing a color value in packages/cache/tests/hydration.js.

According to this comment you should try to add a new .toMatchInlineSnapshot() (without any arguments) to a file. It might also be important to use a specific file as it's dependent on what kind of syntax a particular file contains. Given that a good candidate for testing would be the one from that mentioned PR: packages/react/__tests__/custom-cache.js

Since the patch has been re-added there isn't a strong need to spend time verifying if it's needed though.

@Andarist Andarist merged commit 7b4f016 into emotion-js:main Aug 2, 2022
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.

None yet

2 participants