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

Fix broken yarn pnp #32867

Merged
merged 7 commits into from Jan 11, 2022
Merged

Fix broken yarn pnp #32867

merged 7 commits into from Jan 11, 2022

Conversation

kaykdm
Copy link
Contributor

@kaykdm kaykdm commented Dec 28, 2021

x-ref #31552
x-ref #32115
x-ref #32546
x-ref #32721

Since this PR #31455 is merged, enhanced-resolve dependency's resolved field is changed which caused broken yarn pnp.
I am not sure how this field has been changed or this is intentional or not
When I install webpack locally, enhanced-resolve's resolved field in lock file is always registry.yarnpkg.com not codeload.github.com

Bug

  • Related issues linked using fixes #number
  • Integration tests added
  • Errors have helpful link attached, see contributing.md

Feature

  • Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
  • Related issues linked using fixes #number
  • Integration tests added
  • Documentation added
  • Telemetry added. In case of a feature if it's used or not.
  • Errors have helpful link attached, see contributing.md

Documentation / Examples

  • Make sure the linting passes by running yarn lint

@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@grefrit
Copy link

grefrit commented Dec 28, 2021

Can't wait for this, because of broken PnP we have to stay on Next 11. Thanks a lot)

@rtritto
Copy link

rtritto commented Dec 28, 2021

If this PR fixes #31812, should #31995 be reverted/removed?

FYI @sokra

@kaykdm
Copy link
Contributor Author

kaykdm commented Dec 28, 2021

If this PR fixes #31812, should #31995 be reverted/removed?

FYI @sokra

@rtritto
oh, I am sorry for not being specific .
This PR does not fix #31812. You still need to set swcFileReading: false to be able to run in pnp.
I am gonna remove the issue link.
Thanks

@kaykdm kaykdm changed the title fix broken yarn pnp Fix broken yarn pnp Dec 29, 2021
@ijjk

This comment has been minimized.

sokra
sokra previously approved these changes Jan 11, 2022
Copy link
Member

@sokra sokra left a comment

Choose a reason for hiding this comment

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

This is definitely invalid in our lockfile, but afaik our lockfile doesn't affect next.js users at all. So I don't think this will fix any problem.

@ijjk

This comment has been minimized.

ijjk
ijjk previously approved these changes Jan 11, 2022
@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Jan 11, 2022

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall decrease ✓
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
buildDuration 14s 14s ⚠️ +36ms
buildDurationCached 3.1s 3.1s ⚠️ +20ms
nodeModulesSize 355 MB 355 MB -8.75 kB
Page Load Tests Overall increase ✓
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
/ failed reqs 0 0
/ total time (seconds) 2.827 2.861 ⚠️ +0.03
/ avg req/sec 884.24 873.85 ⚠️ -10.39
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.293 1.277 -0.02
/error-in-render avg req/sec 1933.16 1957.98 +24.82
Client Bundles (main, webpack, commons)
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 27.1 kB 27.1 kB
webpack-HASH.js gzip 1.45 kB 1.45 kB
Overall change 70.9 kB 70.9 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
_app-HASH.js gzip 1.37 kB 1.37 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.37 kB 2.37 kB
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 919 B 919 B
image-HASH.js gzip 4.74 kB 4.74 kB
index-HASH.js gzip 263 B 263 B
link-HASH.js gzip 2.13 kB 2.13 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.1 kB 14.1 kB
Client Build Manifests
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
_buildManifest.js gzip 459 B 459 B
Overall change 459 B 459 B
Rendered Page Sizes
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
index.html gzip 533 B 533 B
link.html gzip 547 B 547 B
withRouter.html gzip 528 B 528 B
Overall change 1.61 kB 1.61 kB

Default Build with SWC (Decrease detected ✓)
General Overall decrease ✓
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
buildDuration 15.3s 15.5s ⚠️ +220ms
buildDurationCached 3.1s 3.1s ⚠️ +39ms
nodeModulesSize 355 MB 355 MB -8.75 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
/ failed reqs 0 0
/ total time (seconds) 2.836 2.869 ⚠️ +0.03
/ avg req/sec 881.56 871.27 ⚠️ -10.29
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.28 1.283 0
/error-in-render avg req/sec 1953.19 1949.04 ⚠️ -4.15
Client Bundles (main, webpack, commons)
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.3 kB 42.3 kB
main-HASH.js gzip 27.2 kB 27.2 kB
webpack-HASH.js gzip 1.44 kB 1.44 kB
Overall change 71.1 kB 71.1 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error 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.36 kB 2.36 kB
head-HASH.js gzip 342 B 342 B
hooks-HASH.js gzip 906 B 906 B
image-HASH.js gzip 4.76 kB 4.76 kB
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 2.19 kB 2.19 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.1 kB 14.1 kB
Client Build Manifests
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
_buildManifest.js gzip 458 B 458 B
Overall change 458 B 458 B
Rendered Page Sizes
vercel/next.js canary kaykdm/next.js fix/yarn-pnp-error Change
index.html gzip 532 B 532 B
link.html gzip 545 B 545 B
withRouter.html gzip 526 B 526 B
Overall change 1.6 kB 1.6 kB
Commit: 0739887

@kodiakhq kodiakhq bot merged commit 2d0fd34 into vercel:canary Jan 11, 2022
@kachkaev
Copy link
Contributor

Keen to test this with Yarn 3 once a new canary is out!

@balazsorban44
Copy link
Member

There is @kachkaev, please check out 12.0.8-canary.21 and report back in a new issue if something is wrong! 🙏

@kachkaev
Copy link
Contributor

kachkaev commented Jan 11, 2022

I just bumped Next to 12.0.8-canary.21 in kachkaev/njt#161. Happy to confirm that yarn dev, yarn build and yarn start work again for me! 🎉 Huge thanks to @kaykdm for this PR!

@rauchg
Copy link
Member

rauchg commented Jan 11, 2022

How can we set up automated testing to avoid this kind of regression?

@kaykdm kaykdm deleted the fix/yarn-pnp-error branch January 12, 2022 02:00
@kachkaev
Copy link
Contributor

kachkaev commented Jan 12, 2022

Yeah having some tests would be nice. I just upgraded from "next": "^12.0.8-canary.21" to "next": "^12.0.8" and cryptic Yarn PnP errors went back 😅

yarn dev

ready - started server on 0.0.0.0:3000, url: http://localhost:3000
warn  - You have enabled experimental feature(s).
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

error - ./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js
Error: Failed to read source code from /path/to/project/.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js

Caused by:
    No such file or directory (os error 2)
wait  - compiling...
error - ./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js
Error: Failed to read source code from /path/to/project/.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js

Caused by:
    No such file or directory (os error 2)
wait  - compiling...
error - ./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js
Error: Failed to read source code from /path/to/project/.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/performance-relayer.js
yarn build

warn  - You have enabled experimental feature(s).
warn  - Experimental features are not covered by semver, and may cause unexpected or broken application behavior. Use them at your own risk.

info  - Skipping validation of types  
info  - Creating an optimized production build  
Failed to compile.

./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/portal/index.js
Error: Failed to read source code from /path/to/project/.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/portal/index.js

Caused by:
    No such file or directory (os error 2)

Import trace for requested module:
./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/index.js
./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/next.js

./.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/script.js
Error: Failed to read source code from /path/to/project/.yarn/__virtual__/next-virtual-1575485aa2/0/cache/next-npm-12.0.8-aa55acca00-947f0295aa.zip/node_modules/next/dist/client/script.js

Let’s create a new issue if anyone else can confirm. You can try it in kachkaev/njt#161 by upgrading next and @next/eslint-plugin-next in package.json.

What’s interesting is that yarn dev is able to work after a couple of attempts, so there might be something with timing. yarn build fails 100% of times in 12.0.8 though.

@kaykdm
Copy link
Contributor Author

kaykdm commented Jan 12, 2022

@kachkaev
I just tried "next": "^12.0.8", and it is working for me.
Did you set swcFileReading: false in your next.config.js ?

@kachkaev
Copy link
Contributor

kachkaev commented Jan 12, 2022

Good shout @kaykdm! I had swcFileReading: false when testing 12.0.8-canary.21. Then I removed the option, ran the commands again and they were still OK (probably because of caching).

I can confirm that 12.0.8 works when swcFileReading: false is present in next.config.mjs 🎉

Not sure how difficult it would be to support swcFileReading but I wonder if Next could detect Yarn PnP and show a nice error instead of the cryptic ones. I wish I could submit a PR, but I currently don’t have enough time to implement this little DX improvement.

@brc-dd
Copy link

brc-dd commented Jan 12, 2022

How can we set up automated testing to avoid this kind of regression?

@rauchg You can just check if there is any match for /:\/\/(?!registry.yarnpkg.com)/g in your lockfile. 😄

There are two such matches right now BTW. Also, I believe that setting registry "https://registry.yarnpkg.com/" in .yarnrc at project root can reduce such issues in the future.

teleaziz added a commit to teleaziz/next.js that referenced this pull request Jan 12, 2022
…o-example

* 'canary' of github.com:vercel/next.js:
  Added links to data fetching api refs, fixed title (vercel#33221)
  Removed backticks on data fetching api titles (vercel#33216)
  middlewares: limit `process.env` to inferred usage (vercel#33186)
  Fixed broken link (vercel#33209)
  v12.0.8
  v12.0.8-canary.22
  Refactor data fetching API docs (vercel#30615)
  Docs: correct ignorance pattern for .env.local (vercel#32647)
  Fixes vercel#33153: Updating cross-references from master to main + canary (vercel#33198)
  v12.0.8-canary.21
  Add util for normalizing errors (vercel#33159)
  Fix broken yarn pnp (vercel#32867)
@ijjk
Copy link
Member

ijjk commented Jan 12, 2022

Regression tests for this are being added in #33236

@vercel vercel locked as resolved and limited conversation to collaborators Feb 12, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants