chore: fix e2e yarn berry tests #5342

slorber · 2021-08-11T18:13:38Z

Motivation

The Yarn 2 CI is failing for a while now and it's quite difficult to debug those Yarn 2/3 PnP-only issues.

For now, I'm reverting to node-modules linking, but would be happy to add another test with PnP mode if anyone is able to make it work.

CC @SamChou19815 in case you are interested to troubleshoot the issue. I was not able to do so (#5340).

Repro steps:

rm -rf my-website-canary

npx @docusaurus/init@canary init my-website-canary classic --skip-install

cd my-website-canary


rm -rf node_modules yarn.lock .yarn .yarnrc.yml
yarn -v

yarn set version berry
yarn -v

yarn set version 2.4.2
yarn -v

yarn config set pnpMode loose
yarn config set enableGlobalCache true

yarn install

yarn start

Apart a few deps error, the main error is:

Watchpack Error (initial scan): TypeError: Cannot destructure property 'withFileTypes' of '(intermediate value)(intermediate value)(intermediate value)' as it is null.

Unfortunately can't figure out where this comes from, and the PnP zip files do not help debug this problem.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

ci

netlify · 2021-08-11T18:19:24Z

✔️ [V2]

🔨 Explore the source changes: 8422f9a

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61150065e5ade900074f8748

😎 Browse the preview: https://deploy-preview-5342--docusaurus-2.netlify.app

github-actions · 2021-08-11T18:19:37Z

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟢 Performance	96
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

Lighthouse ran on https://deploy-preview-5342--docusaurus-2.netlify.app/

github-actions · 2021-08-11T18:22:42Z

Size Change: 0 B

Total Size: 793 kB

ℹ️ View Unchanged

Filename	Size
`website/.docusaurus/globalData.json`	36.2 kB
`website/build/assets/css/styles.********.css`	93.1 kB
`website/build/assets/js/main.********.js`	403 kB
`website/build/blog/2017/12/14/introducing-docusaurus/index.html`	64.2 kB
`website/build/blog/index.html`	33.1 kB
`website/build/docs/index.html`	41.9 kB
`website/build/docs/installation/index.html`	49.6 kB
`website/build/index.html`	28.7 kB
`website/build/tests/docs/index.html`	22.5 kB
`website/build/tests/docs/standalone/index.html`	20.5 kB

_{compressed-size-action}

SamChou19815 · 2021-08-11T19:04:42Z

@slorber The withFileTypes error seems to be an known issue that is just fixed a few days ago, but Yarn hasn't released a new stable version yet: yarnpkg/berry#3234

Maybe we can merge this now or test with canary versions of Yarn.

.github/workflows/v2-tests-e2e.yml

merceyz · 2021-08-12T10:27:28Z

.github/workflows/v2-tests-e2e.yml

+          yarn set version canary
+          yarn --version
+
+          yarn config set nodeLinker ${{ matrix.nodeLinker }}
          yarn config set pnpMode loose


Suggested change

yarn config set pnpMode loose

There are undeclared dependencies that aren't failing the test because of this

Yes I've seen those.
However some are:

mdx loader (my fault, will fix)

debug using supports-color (not my fault, how to fix?)

code copied from CRA using Node.js url. We'll likely delete this code and move back to using CRA code (which also doesn't declare url as dependency btw). Also I'm not totally sure how to fix this. Should I just add npm package url as dependency? How can I know the good version to use considering it's provided by Node.js runtime?

I'd like to fix those but likely in another PR.
The goal here is just to make our existing CI pass again (which was already using loose mode)

debug using supports-color (not my fault, how to fix?)

Nothing to fix, it's an optional peer dependency that isn't provided by the parent so it logs an error but it's fine to ignore and wont be visible in strict mode

code copied from CRA using Node.js url. We'll likely delete this code and move back to using CRA code (which also doesn't declare url as dependency btw). Also I'm not totally sure how to fix this. Should I just add npm package url as dependency? How can I know the good version to use considering it's provided by Node.js runtime?

It's because Webpack 5 doesn't polyfill Node libraries by default, you need to do it yourself

Nothing to fix, it's an optional peer dependency that isn't provided by the parent so it logs an error but it's fine to ignore and wont be visible in strict mode

Thanks

It's because Webpack 5 doesn't polyfill Node libraries by default, you need to do it yourself

I've tried adding alias: {url: 'url'} in Webpack config but the error was still there. It's not clear to me what I should do exactly and the Medium link you gave in some yarn issue didn't make it very clear either

You're running into https://github.com/webpack/changelog-v5/tree/b9a346f9090bf2e9f8bc96864a88ed07ad7bc1a4#automatic-nodejs-polyfills-removed so aliasing url to url makes no difference.

Next.js deals with this by aliasing url to native-url https://github.com/vercel/next.js/blob/681d298bdf7048bdbd2790a551d5d231906ede6f/packages/next/build/webpack-config.ts#L125

I see thanks, will try that.
For the mdx-loader issue, unfortunately I can't just add it because it would create a cycle in Lerna build:

lerna WARN ECYCLE @docusaurus/mdx-loader -> @docusaurus/core -> @docusaurus/mdx-loader

I see how I could re-org the code to avoid this cycle but was wondering if there's a way to just tell Yarn that the dependency is here without having to declare it in any deps? peerDependency works but is annoying because all site's parent lockfile would now have to declare @docusaurus/mdx-loader, while we know for sure it is here.

Co-authored-by: Kristoffer K. <merceyz@users.noreply.github.com>

slorber · 2021-08-12T11:17:48Z

Thanks all, finally fixing this annoying issue!

We'll revert to a stable release, and try to disable loose mode in the future.

As Yarn 3 is out, I don't feel like we need to have tests for Yarn 2 anymore, probably not worth it to add more 2 more CI jobs, we already have a lot

merceyz · 2021-08-12T11:18:34Z

.github/workflows/v2-tests-e2e.yml

+          # temporary using canary for #5342
+          yarn set version canary


We've released 3.0.1 with the fix so this can be reverted

ohhh too late :D

anyway I don't mind staying on canary for now, I could report issues we have with it that do not happen in stable releases

disable pnp

dabd702

facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 11, 2021

slorber added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Aug 11, 2021

merceyz reviewed Aug 11, 2021

View reviewed changes

.github/workflows/v2-tests-e2e.yml Show resolved Hide resolved

slorber added 4 commits August 12, 2021 11:35

test-release support --skip-install + revert to pnp mode

a77f706

fix yarn canary?

2590a13

add YARN_ENABLE_IMMUTABLE_INSTALLS env

4d64db6

add nodeLinker matrix

22f4c33

merceyz reviewed Aug 12, 2021

View reviewed changes

slorber changed the title ~~chore: e2e yarn berry tests: disable pnp~~ chore: e2e yarn berry tests Aug 12, 2021

slorber changed the title ~~chore: e2e yarn berry tests~~ chore: fix e2e yarn berry tests Aug 12, 2021

slorber mentioned this pull request Aug 12, 2021

chore: e2e tests should stay on Yarn v2 for now #5340

Closed

slorber and others added 4 commits August 12, 2021 12:38

Update .github/workflows/v2-tests-e2e.yml

9dc6a59

Co-authored-by: Kristoffer K. <merceyz@users.noreply.github.com>

polish e2e test workflows

b07199c

polish e2e test workflows

70bcb09

set npm_config_registry

8422f9a

slorber merged commit ac4a253 into master Aug 12, 2021

slorber deleted the slorber/disable-pnp branch August 12, 2021 11:18

merceyz reviewed Aug 12, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: fix e2e yarn berry tests #5342

chore: fix e2e yarn berry tests #5342

slorber commented Aug 11, 2021 •

edited

netlify bot commented Aug 11, 2021 •

edited

github-actions bot commented Aug 11, 2021 •

edited

github-actions bot commented Aug 11, 2021 •

edited

SamChou19815 commented Aug 11, 2021

merceyz Aug 12, 2021 •

edited

slorber Aug 12, 2021

slorber Aug 12, 2021

merceyz Aug 12, 2021

slorber Aug 12, 2021

merceyz Aug 12, 2021

slorber Aug 12, 2021

slorber commented Aug 12, 2021

merceyz Aug 12, 2021

slorber Aug 12, 2021

chore: fix e2e yarn berry tests #5342

chore: fix e2e yarn berry tests #5342

Conversation

slorber commented Aug 11, 2021 • edited

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

netlify bot commented Aug 11, 2021 • edited

github-actions bot commented Aug 11, 2021 • edited

github-actions bot commented Aug 11, 2021 • edited

SamChou19815 commented Aug 11, 2021

merceyz Aug 12, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber commented Aug 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slorber commented Aug 11, 2021 •

edited

netlify bot commented Aug 11, 2021 •

edited

github-actions bot commented Aug 11, 2021 •

edited

github-actions bot commented Aug 11, 2021 •

edited

merceyz Aug 12, 2021 •

edited