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

Allow Yarn PnP to access internal pages #23374

Closed
wants to merge 1 commit into from

Conversation

strothj
Copy link

@strothj strothj commented Mar 25, 2021

Bug

  • Related issues linked using fixes #number
  • Integration tests added

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.

Documentation / Examples

  • Make sure the linting passes

This pull request fixes #21828.

While operating under a Yarn Plug'n'Play environment, dependencies are stored within .zip files and no node_modules directory exists.

When attempting to access a page that does not exist, the browser would display a loading spinner and never finish. This is because the internal error page was determined to not exist.

The server hot reloader was checking if a page exists by seeing if write access was granted here. I replaced the write access check with a read access test.

I'm not sure if an integration test is required in this case or how one would go about writing one to cover this. The existing integration tests appear unaffected by this change.

@ijjk
Copy link
Member

ijjk commented Mar 25, 2021

Stats from current PR

Default Server Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
buildDuration 17.1s 17.2s ⚠️ +151ms
nodeModulesSize 61.7 MB 61.7 MB ⚠️ +1.16 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
/ failed reqs 0 0
/ total time (seconds) 2.261 2.349 ⚠️ +0.09
/ avg req/sec 1105.88 1064.07 ⚠️ -41.81
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.508 1.583 ⚠️ +0.07
/error-in-render avg req/sec 1658.11 1578.92 ⚠️ -79.19
Client Bundles (main, webpack, commons)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
597-ec2335c0..e105.js gzip 13.3 kB 13.3 kB
778-b85ea97d..1a55.js gzip 7.06 kB 7.06 kB
framework.HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 151 B 151 B
webpack-HASH.js gzip 993 B 993 B
Overall change 60.8 kB 60.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_app-bdbd9e6..6cfe.js gzip 1.3 kB 1.3 kB
_error-b58c1..9b8e.js gzip 3.4 kB 3.4 kB
amp-89a5460c..567f.js gzip 558 B 558 B
hooks-8c2e74..be37.js gzip 924 B 924 B
index-fec729..83b2.js gzip 243 B 243 B
link-d124373..c521.js gzip 1.66 kB 1.66 kB
routerDirect..5759.js gzip 336 B 336 B
withRouter-1..98bf.js gzip 334 B 334 B
Overall change 8.76 kB 8.76 kB
Client Build Manifests
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_buildManifest.js gzip 323 B 323 B
Overall change 323 B 323 B
Rendered Page Sizes
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
index.html gzip 608 B 608 B
link.html gzip 615 B 615 B
withRouter.html gzip 603 B 603 B
Overall change 1.83 kB 1.83 kB

Serverless Mode
General Overall increase ⚠️
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
buildDuration 21s 20.9s -100ms
nodeModulesSize 61.7 MB 61.7 MB ⚠️ +1.16 kB
Client Bundles (main, webpack, commons)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
597-ec2335c0..e105.js gzip 13.3 kB 13.3 kB
778-b85ea97d..1a55.js gzip 7.06 kB 7.06 kB
framework.HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 151 B 151 B
webpack-HASH.js gzip 993 B 993 B
Overall change 60.8 kB 60.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_app-bdbd9e6..6cfe.js gzip 1.3 kB 1.3 kB
_error-b58c1..9b8e.js gzip 3.4 kB 3.4 kB
amp-89a5460c..567f.js gzip 558 B 558 B
hooks-8c2e74..be37.js gzip 924 B 924 B
index-fec729..83b2.js gzip 243 B 243 B
link-d124373..c521.js gzip 1.66 kB 1.66 kB
routerDirect..5759.js gzip 336 B 336 B
withRouter-1..98bf.js gzip 334 B 334 B
Overall change 8.76 kB 8.76 kB
Client Build Manifests
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_buildManifest.js gzip 323 B 323 B
Overall change 323 B 323 B
Serverless bundles
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_error.js 1.34 MB 1.34 MB
404.html 2.76 kB 2.76 kB
500.html 2.75 kB 2.75 kB
amp.amp.html 10.6 kB 10.6 kB
amp.html 1.96 kB 1.96 kB
hooks.html 2.01 kB 2.01 kB
index.js 1.34 MB 1.34 MB
link.js 1.4 MB 1.4 MB
routerDirect.js 1.39 MB 1.39 MB
withRouter.js 1.39 MB 1.39 MB
Overall change 6.89 MB 6.89 MB

Webpack 5 Mode (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
buildDuration 17s 17.4s ⚠️ +421ms
nodeModulesSize 61.7 MB 61.7 MB ⚠️ +1.16 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
/ failed reqs 0 0
/ total time (seconds) 2.299 2.273 -0.03
/ avg req/sec 1087.29 1099.72 +12.43
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.53 1.578 ⚠️ +0.05
/error-in-render avg req/sec 1634.18 1584.05 ⚠️ -50.13
Client Bundles (main, webpack, commons)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
597-ec2335c0..e105.js gzip 13.3 kB 13.3 kB
778-b85ea97d..1a55.js gzip 7.06 kB 7.06 kB
framework.HASH.js gzip 39.3 kB 39.3 kB
main-HASH.js gzip 151 B 151 B
webpack-HASH.js gzip 993 B 993 B
Overall change 60.8 kB 60.8 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
polyfills-HASH.js gzip 31.1 kB 31.1 kB
Overall change 31.1 kB 31.1 kB
Client Pages
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_app-bdbd9e6..6cfe.js gzip 1.3 kB 1.3 kB
_error-b58c1..9b8e.js gzip 3.4 kB 3.4 kB
amp-89a5460c..567f.js gzip 558 B 558 B
hooks-8c2e74..be37.js gzip 924 B 924 B
index-fec729..83b2.js gzip 243 B 243 B
link-d124373..c521.js gzip 1.66 kB 1.66 kB
routerDirect..5759.js gzip 336 B 336 B
withRouter-1..98bf.js gzip 334 B 334 B
Overall change 8.76 kB 8.76 kB
Client Build Manifests
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
_buildManifest.js gzip 323 B 323 B
Overall change 323 B 323 B
Rendered Page Sizes
vercel/next.js canary strothj/next.js patch-pnp-compatibility Change
index.html gzip 608 B 608 B
link.html gzip 615 B 615 B
withRouter.html gzip 603 B 603 B
Overall change 1.83 kB 1.83 kB

Diffs

Diff for index.html
@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/",
         "query": {},
-        "buildId": "tf0ffP6jioT27LO2NRL1Z",
+        "buildId": "gvXMmkZAXF75MMpRZofPj",
         "isFallback": false,
         "gip": true
       }
@@ -86,11 +86,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_buildManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_ssgManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for link.html
@@ -53,7 +53,7 @@
         "props": { "pageProps": {} },
         "page": "/link",
         "query": {},
-        "buildId": "tf0ffP6jioT27LO2NRL1Z",
+        "buildId": "gvXMmkZAXF75MMpRZofPj",
         "isFallback": false,
         "gip": true
       }
@@ -91,11 +91,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_buildManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_ssgManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_ssgManifest.js"
       async=""
     ></script>
   </body>
Diff for withRouter.html
@@ -48,7 +48,7 @@
         "props": { "pageProps": {} },
         "page": "/withRouter",
         "query": {},
-        "buildId": "tf0ffP6jioT27LO2NRL1Z",
+        "buildId": "gvXMmkZAXF75MMpRZofPj",
         "isFallback": false,
         "gip": true
       }
@@ -86,11 +86,11 @@
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_buildManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_buildManifest.js"
       async=""
     ></script>
     <script
-      src="/_next/static/tf0ffP6jioT27LO2NRL1Z/_ssgManifest.js"
+      src="/_next/static/gvXMmkZAXF75MMpRZofPj/_ssgManifest.js"
       async=""
     ></script>
   </body>
Commit: e40af37

@strothj
Copy link
Author

strothj commented Mar 25, 2021

I believe the failing PnP test is related to this: #23354

@merceyz
Copy link
Contributor

merceyz commented Apr 22, 2021

The PnP e2e test is passing on canary again (#24216)

@strothj strothj closed this Jun 1, 2021
@vercel vercel locked as resolved and limited conversation to collaborators Jan 28, 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.

next dev will not resolve missing routes using yarn 2 zero-install
3 participants