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

Switch to Yarn 4 (Berry) #680

Draft
wants to merge 60 commits into
base: main
Choose a base branch
from
Draft

Switch to Yarn 4 (Berry) #680

wants to merge 60 commits into from

Conversation

kachkaev
Copy link
Contributor

@kachkaev kachkaev commented Oct 18, 2022

🌟 What is the purpose of this PR?

This PR migrates this repo to Yarn Berry (a.k.a. Yarn Modern) by introducing a local version of Yarn (yarn set version canary).

🔗 Related links

🚫 Blocked by

🔍 What does this change?

  • Slightly faster yarn install
  • Unlocked access to Yarn PnP for much faster install times
  • Unlocked access to Yarn plugins and new commands (see yarn help)
  • Stricter resolution of node modules (a package is no longer discoverable if it is absent in workspace dependencies)
  • Synchronised upgradable Yarn version for the whole team (it’s picked from .yarn/releases for everyone)
  • yarn.lock is a valid YAML now (this change is the largest contributor to PR diff)
  • + see PR comments

📜 Does this require a change to the docs?

  • No because commands have not changed. Users still need to install Yarn v1 globally as before. Yarn Classic checks if there is a local Yarn binary and delegates everything to it, if found.

⚠️ Known issues

  • As of this PR's opening, Yarn 4 was still RC and subject to breaking changes. However, since October 2023 4.x has been official and stable.

  • @kachkaev: Yarn Berry has a built-in yarn patch functionality, but these patches are not automatically upgraded: [Bug?]: Yarn upgrade doesn't upgrade patches  yarnpkg/berry#4970. I've kept patch-package for now, but we should be able to remove this extra dependency one day and end up with .yarn/patches instead of patches in root.

  • @kachkaev: I had to add "lib": ["DOM", "DOM.Iterable", "ESNext"] to the base Node.js config. This was needed for type-checking node_modules as part of lint:tsc. The problem is with some Node-only dependencies depending on typings from DOM lib. We could get away with these upstream bugs because package resolution in Yarn 1 is a bit more loose.

🐾 Next steps

🛡 What tests cover this?

  • CI

@github-actions github-actions bot added area: dependencies Relates to third-party or otherwise imported dependencies (area) area: infra Relates to version control, CI, CD or IaC (area) area: packages area: libs > @local/* Affects packages in the `@local` namespace (library) area: libs > block-template-html Affects the `block-template-html` package (library) labels Oct 18, 2022
@github-actions github-actions bot added area: apps > site The blockprotocol.org website, inc. Hub (app) area: apps Relates to non-content work in `apps` (area) area: libs > mock-block-dock Affects the `mock-block-dock` package (library) labels Oct 18, 2022
@codecov
Copy link

codecov bot commented Oct 18, 2022

Codecov Report

Merging #680 (da95d14) into main (41e1046) will decrease coverage by 55.77%.
The diff coverage is n/a.

❗ Current head da95d14 differs from pull request most recent head 630b06e. Consider uploading reports for the commit 630b06e to get more accurate results

@@            Coverage Diff            @@
##             main   #680       +/-   ##
=========================================
- Coverage   55.76%      0   -55.77%     
=========================================
  Files         401      0      -401     
  Lines        5986      0     -5986     
  Branches     1312      0     -1312     
=========================================
- Hits         3338      0     -3338     
+ Misses       2268      0     -2268     
+ Partials      380      0      -380     
Flag Coverage Δ
site-integration-chrome ?
site-integration-firefox ?
site-integration-iphone ?
site-integration-pixel ?
site-integration-safari ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...rc/components/pages/pricing/custom-link-button.tsx
apps/site/src/lib/s3.ts
apps/site/src/components/icons/arrow-up-icon.tsx
...nts/pages/user/tab-panel-contents-with-schemas.tsx
apps/site/src/lib/config.ts
apps/site/src/components/icons/pro/fa-lock.tsx
apps/site/src/theme/shadows.ts
...pps/site/src/components/icons/arrow-right-icon.tsx
apps/site/src/components/icons/pro/fa-coins.tsx
...ashboard/dashboard-card/dashboard-card-loading.tsx
... and 391 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@kachkaev kachkaev changed the title Upgrade Yarn Switch to Yarn 4 (Berry) Oct 19, 2022
@github-actions github-actions bot removed the area: user success > docs Þ Documentation: blockprotocol.org/docs (content) label Jan 10, 2023
@@ -8,28 +8,28 @@ const monorepoRoot = path.resolve(__dirname, "../../..");
* Because .*ignore files are not composable, we cannot import or otherwise reuse
* a top-level .eslintignore. To avoid repetition and to maintain a coherent behavior
* of ESLint CLI and IDE extensions, we generate ignore patterns for each workspace
* based from .gitignore. This is done via ignorePatterns option in ESLint config.
* based from .prettierignore. This is done via ignorePatterns option in ESLint config.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switching ignore list source because of a new section called "Shared between linters". We can ignore .yarn/ in linters, but we still need to track .yarn/releases/* by git.

"lint:eslint": "eslint --report-unused-disable-directives .",
"prepare": "rimraf ./dev/src && lnk -f ./src ./dev",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

prepare is deprecated in Yarn Berry

packages:
name: NPM Packages
if: ${{ github.event_name == 'pull_request' }}
runs-on: ubuntu-20.04
timeout-minutes: 30 ## https://github.com/actions/upload-artifact/issues/358
timeout-minutes: 60
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've increased timeout because cold yarn + rust warmup has been causing CI failures: https://github.com/blockprotocol/blockprotocol/actions/runs/3887049425/jobs/6632856500

We are no longer affected by actions/upload-artifact#358 once #769 has been merged. I've kept a large timeout just in case, but I don't think we need it except for some really rare edge cases.

@vilkinsons
Copy link
Member

vilkinsons commented Jan 11, 2023

Can we get rid of our top-level patches folder in favor of .yarn/patches as part of this PR (or in follow-up?)

It would complement our cleanup of root (started in #875) well.

@nathggns
Copy link
Contributor

@nonparibus it's mentioned in PR why we can't

Yarn Berry has a built-in yarn patch functionality, but these patches are not automatically upgraded: [Bug?]: Yarn upgrade doesn't upgrade patches yarnpkg/berry#4970. I've kept patch-package for now, but we should be able to remove this extra dependency one day and end up with .yarn/patches instead of patches in root.

@vilkinsons
Copy link
Member

I have realized we can specify a custom directory for patch-package allowing root cleanup to be progressed without this PR (see the internal task for more info).

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: apps > site The blockprotocol.org website, inc. Hub (app) area: apps Relates to non-content work in `apps` (area) area: dependencies Relates to third-party or otherwise imported dependencies (area) area: infra Relates to version control, CI, CD or IaC (area) area: libs > block-template-html Affects the `block-template-html` package (library) area: libs > @local/* Affects packages in the `@local` namespace (library) area: libs Relates to first-party libraries/crates/packages (area)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants