-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Lock yarn pnp version in tests #34688
Conversation
@@ -31,7 +31,7 @@ export function runTests(example = '') { | |||
prev.push(`${cur}@${dependencies[cur]}`) | |||
return prev | |||
}, [] as string[]) | |||
return `yarn set version berry && yarn config set enableGlobalCache true && yarn config set compressionLevel 0 && yarn add ${pkgs.join( | |||
return `yarn set version 3.1.1 && yarn config set enableGlobalCache true && yarn config set compressionLevel 0 && yarn add ${pkgs.join( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return `yarn set version 3.1.1 && yarn config set enableGlobalCache true && yarn config set compressionLevel 0 && yarn add ${pkgs.join( | |
return `yarn set version berry && yarn config set enableGlobalCache true && yarn config set compressionLevel 0 && yarn config set pnpEnableEsmLoader false && yarn add ${pkgs.join( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can set this unless it's the default for users with yarn pnp, we want to test the default experience users will be having
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use yarn config set pnpEnableEsmLoader false
since we need Node.js 16 or newer
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
buildDuration | 19s | 19.1s | |
buildDurationCached | 7.6s | 7.6s | -20ms |
nodeModulesSize | 367 MB | 367 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.597 | 3.676 | |
/ avg req/sec | 694.93 | 680.13 | |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.795 | 1.713 | -0.08 |
/error-in-render avg req/sec | 1392.52 | 1459.74 | +67.22 |
Client Bundles (main, webpack)
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42 kB | 42 kB | ✓ |
main-HASH.js gzip | 27.8 kB | 27.8 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.5 kB | 71.5 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.36 kB | 1.36 kB | ✓ |
_error-HASH.js gzip | 194 B | 194 B | ✓ |
amp-HASH.js gzip | 312 B | 312 B | ✓ |
css-HASH.js gzip | 326 B | 326 B | ✓ |
dynamic-HASH.js gzip | 2.57 kB | 2.57 kB | ✓ |
head-HASH.js gzip | 350 B | 350 B | ✓ |
hooks-HASH.js gzip | 919 B | 919 B | ✓ |
image-HASH.js gzip | 5.05 kB | 5.05 kB | ✓ |
index-HASH.js gzip | 263 B | 263 B | ✓ |
link-HASH.js gzip | 2.26 kB | 2.26 kB | ✓ |
routerDirect..HASH.js gzip | 321 B | 321 B | ✓ |
script-HASH.js gzip | 383 B | 383 B | ✓ |
withRouter-HASH.js gzip | 318 B | 318 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.7 kB | 14.7 kB | ✓ |
Client Build Manifests
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
_buildManifest.js gzip | 460 B | 460 B | ✓ |
Overall change | 460 B | 460 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
index.html gzip | 530 B | 530 B | ✓ |
link.html gzip | 544 B | 544 B | ✓ |
withRouter.html gzip | 525 B | 525 B | ✓ |
Overall change | 1.6 kB | 1.6 kB | ✓ |
Default Build with SWC (Increase detected ⚠️ )
General Overall increase ⚠️
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
buildDuration | 23.1s | 22.8s | -321ms |
buildDurationCached | 7.6s | 7.5s | -89ms |
nodeModulesSize | 367 MB | 367 MB |
Page Load Tests Overall increase ✓
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
/ failed reqs | 0 | 0 | ✓ |
/ total time (seconds) | 3.604 | 3.566 | -0.04 |
/ avg req/sec | 693.76 | 701.06 | +7.3 |
/error-in-render failed reqs | 0 | 0 | ✓ |
/error-in-render total time (seconds) | 1.675 | 1.654 | -0.02 |
/error-in-render avg req/sec | 1492.46 | 1511.35 | +18.89 |
Client Bundles (main, webpack)
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
450.HASH.js gzip | 179 B | 179 B | ✓ |
framework-HASH.js gzip | 42.1 kB | 42.1 kB | ✓ |
main-HASH.js gzip | 27.8 kB | 27.8 kB | ✓ |
webpack-HASH.js gzip | 1.44 kB | 1.44 kB | ✓ |
Overall change | 71.6 kB | 71.6 kB | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 31 kB | 31 kB | ✓ |
Overall change | 31 kB | 31 kB | ✓ |
Client Pages
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
_app-HASH.js gzip | 1.35 kB | 1.35 kB | ✓ |
_error-HASH.js gzip | 180 B | 180 B | ✓ |
amp-HASH.js gzip | 305 B | 305 B | ✓ |
css-HASH.js gzip | 321 B | 321 B | ✓ |
dynamic-HASH.js gzip | 2.56 kB | 2.56 kB | ✓ |
head-HASH.js gzip | 342 B | 342 B | ✓ |
hooks-HASH.js gzip | 911 B | 911 B | ✓ |
image-HASH.js gzip | 5.08 kB | 5.08 kB | ✓ |
index-HASH.js gzip | 256 B | 256 B | ✓ |
link-HASH.js gzip | 2.28 kB | 2.28 kB | ✓ |
routerDirect..HASH.js gzip | 314 B | 314 B | ✓ |
script-HASH.js gzip | 375 B | 375 B | ✓ |
withRouter-HASH.js gzip | 309 B | 309 B | ✓ |
85e02e95b279..7e3.css gzip | 107 B | 107 B | ✓ |
Overall change | 14.7 kB | 14.7 kB | ✓ |
Client Build Manifests
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
_buildManifest.js gzip | 459 B | 459 B | ✓ |
Overall change | 459 B | 459 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | ijjk/next.js test/lock-yarn-berry | Change | |
---|---|---|---|
index.html gzip | 534 B | 534 B | ✓ |
link.html gzip | 547 B | 547 B | ✓ |
withRouter.html gzip | 528 B | 528 B | ✓ |
Overall change | 1.61 kB | 1.61 kB | ✓ |
As discussed in slack I don't think we should change of config in our tests for now to ensure a breaking change isn't being made in PnP which makes it appear to be Next.js fault when it's not. Let's start with locking the version here to unblock other PRs though. |
For context, the core issue isn't from Next.js, nor Yarn (nor even Jest). Node <16.14 have a bug where, when using The problem appeared with Yarn 3.2 because it enables ESM support in more situations than before, which causes loaders to be added to the environment, and thus triggers the Node bug. Now, considering that:
We currently don't plan to revert the ESM change. May change if we hear more problems, of course. |
@arcanis in the latest version of Yarn why is ESM being used to load Next.js or |
Because |
Shouldn't only those specific packages be loaded with ESM by yarn and also only if loaded via |
No; ESM packages can be imported from commonjs code using the |
So just having the ESM loader configured and not even necessarily being used for |
As I mentioned it's not a bug in Yarn; we use the APIs provided. But for the other points yes: having any ESM loader set (PnP or otherwise, such as |
If this isn't specific to Yarn berry can we reproduce it with |
Yes; it's a little more awkward due to another Node bug that the Yarn loader workarounds (having a loader configured currently forces the entry point to be evaluated in ESM mode, thus preventing the Next binary to run at all), but the same can be reproduced with npm with a little help: next.mjs import {createRequire} from 'module';
createRequire(import.meta.url)('next/dist/bin/next'); command line node --experimental-loader ./path-to-an-empty-file.mjs ./next.mjs build |
It's possible to workaround the issue in userland by having the child process send a "ready" message before the parent starts sending messages to it. |
It seems the most recent publish for yarn berry is causing a breaking change with
jest-worker
breaking tests. This temporarily locks the version we test against until this is resolved.x-ref: https://github.com/vercel/next.js/runs/5290374235?check_suite_focus=true
x-ref: https://github.com/vercel/next.js/runs/5289675273?check_suite_focus=true
x-ref: https://github.com/vercel/next.js/runs/5282660046?check_suite_focus=true