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

[Do not merge] Initial fix of #30549 #30654

Closed
wants to merge 1 commit into from
Closed

[Do not merge] Initial fix of #30549 #30654

wants to merge 1 commit into from

Conversation

awareness481
Copy link
Contributor

Bug

Documentation / Examples

  • Make sure the linting passes by running yarn lint

This issue actually occurs due to two separate bugs. First we have next/build/webpack/loaders/next-middleware-ssr-loader/index.ts

The bug occurs because the following code doesn't check whether _app & _document exists but just links to them,

  const stringifiedAbsoluteDocumentPath = getStringifiedAbsolutePath(
    this,
    './pages/_document'
  )
  const stringifiedAbsoluteAppPath = getStringifiedAbsolutePath(
    this,
    './pages/_app'
  )

I've currently added code to just link to the pages in .next as an initial fix.

Doing this fixes the file not found errors but reveals another issue. Upon visiting a route you will get an Error: Circular structure in "getInitialProps" result of page "/rsc" error. The error is thrown from this page _document . However upon reading the actual error, I found out that the issue is that Buffer is not defined. I added a temporary fix for that as well, however I'm not sure what the actual fix is here.

I know that Buffer is provided by webpack in the client but that doesn't seem to work here?

      targetWeb &&
        new webpack.ProvidePlugin({
          Buffer: [require.resolve('buffer'), 'Buffer'],
          process: [require.resolve('process')],
        }),

I opened this PR because I believe I've provided useful information. Hopefully that's the case.

I used the rsc demo to help identify the issue (v12.0.2-canary.11) but note, I would advise to not use yarn next-with-deps as your local environment as it seems to be broken with react 18.

@awareness481
Copy link
Contributor Author

The first part of the issue seems to be worked on on this #30642 but I still believe my explanation on the second part of the issue will be useful. Thanks

@ijjk
Copy link
Member

ijjk commented Oct 30, 2021

Stats from current PR

Default Build (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary awareness481/next.js initial_fix Change
buildDuration 23.6s 23.6s ⚠️ +90ms
buildDurationCached 4.9s 4.9s -7ms
nodeModulesSize 294 MB 294 MB ⚠️ +1.9 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary awareness481/next.js initial_fix Change
/ failed reqs 0 0
/ total time (seconds) 3.959 5.981 ⚠️ +2.02
/ avg req/sec 631.51 417.98 ⚠️ -213.53
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 1.949 1.997 ⚠️ +0.05
/error-in-render avg req/sec 1282.72 1251.93 ⚠️ -30.79
Client Bundles (main, webpack, commons)
vercel/next.js canary awareness481/next.js initial_fix Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 28 kB 28 kB
webpack-HASH.js gzip 1.45 kB 1.45 kB
Overall change 71.9 kB 71.9 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary awareness481/next.js initial_fix Change
polyfills-a4..dd70.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary awareness481/next.js initial_fix Change
_app-HASH.js gzip 1.23 kB 1.23 kB
_error-HASH.js gzip 194 B 194 B
amp-HASH.js gzip 312 B 312 B
css-HASH.js gzip 327 B 327 B
dynamic-HASH.js gzip 2.38 kB 2.38 kB
head-HASH.js gzip 350 B 350 B
hooks-HASH.js gzip 635 B 635 B
image-HASH.js gzip 4.44 kB 4.44 kB
index-HASH.js gzip 263 B 263 B
link-HASH.js gzip 1.87 kB 1.87 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
334f979574ae..6f4.css gzip 106 B 106 B
Overall change 13.1 kB 13.1 kB
Client Build Manifests
vercel/next.js canary awareness481/next.js initial_fix Change
_buildManifest.js gzip 459 B 459 B
Overall change 459 B 459 B
Rendered Page Sizes
vercel/next.js canary awareness481/next.js initial_fix Change
index.html gzip 534 B 534 B
link.html gzip 546 B 546 B
withRouter.html gzip 528 B 528 B
Overall change 1.61 kB 1.61 kB

Default Build with SWC (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary awareness481/next.js initial_fix Change
buildDuration 25s 25.3s ⚠️ +290ms
buildDurationCached 5.1s 4.9s -164ms
nodeModulesSize 294 MB 294 MB ⚠️ +1.9 kB
Page Load Tests Overall decrease ⚠️
vercel/next.js canary awareness481/next.js initial_fix Change
/ failed reqs 0 0
/ total time (seconds) 3.848 6.213 ⚠️ +2.37
/ avg req/sec 649.68 402.39 ⚠️ -247.29
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 2.081 2.023 -0.06
/error-in-render avg req/sec 1201.39 1235.54 +34.15
Client Bundles (main, webpack, commons)
vercel/next.js canary awareness481/next.js initial_fix Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.3 kB 42.3 kB
main-HASH.js gzip 28.2 kB 28.2 kB
webpack-HASH.js gzip 1.43 kB 1.43 kB
Overall change 72.1 kB 72.1 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary awareness481/next.js initial_fix Change
polyfills-a4..dd70.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary awareness481/next.js initial_fix Change
_app-HASH.js gzip 1.22 kB 1.22 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.38 kB 2.38 kB
head-HASH.js gzip 342 B 342 B
hooks-HASH.js gzip 622 B 622 B
image-HASH.js gzip 4.46 kB 4.46 kB
index-HASH.js gzip 256 B 256 B
link-HASH.js gzip 1.91 kB 1.91 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
334f979574ae..6f4.css gzip 106 B 106 B
Overall change 13.1 kB 13.1 kB
Client Build Manifests
vercel/next.js canary awareness481/next.js initial_fix Change
_buildManifest.js gzip 460 B 460 B
Overall change 460 B 460 B
Rendered Page Sizes
vercel/next.js canary awareness481/next.js initial_fix Change
index.html gzip 535 B 535 B
link.html gzip 548 B 548 B
withRouter.html gzip 529 B 529 B
Overall change 1.61 kB 1.61 kB
Commit: 7776754

@kodiakhq kodiakhq bot closed this in #30642 Oct 30, 2021
kodiakhq bot pushed a commit that referenced this pull request Oct 30, 2021
…30642)

* if _app is not provided, fallback to default _app page
* If _document is not provided, fallback to inline functional components version or use the default
* if Document gIP is provided, error

Closes #30654
@huozhi
Copy link
Member

huozhi commented Oct 30, 2021

Thanks for the PR @awareness481

I added new checking that gIP is not allowed when you wanna play with server components and concurrent features #30642. But I didn't get chance to repro the buffer issue you mentioned above. Would you mind provide a minimal reproduction and steps in the issue?

@awareness481
Copy link
Contributor Author

awareness481 commented Oct 31, 2021

Thanks for the PR @awareness481

I added new checking that gIP is not allowed when you wanna play with server components and concurrent features #30642. But I didn't get chance to repro the buffer issue you mentioned above. Would you mind provide a minimal reproduction and steps in the issue?

Hey. The issue is only reproducible if you apply my fix, I can't seem to reproduce it after your fixes. So basically, doing

  // build/webpack/loaders/next-middleware-ssr-loader/index.ts
  const stringifiedAbsoluteDocumentPath = getStringifiedAbsolutePath(
    this,
    '/.next/server/pages/_document'
  )

  const stringifiedAbsoluteAppPath = getStringifiedAbsolutePath(
    this,
    '/.next/server/pages/_app'
  )

will get you a Error: Circular structure in "getInitialProps" result of page "/rsc" error and if you console.log the error you will get a Buffer is not defined error. I'm not sure if this error is something that can appear, given your implementation, going forward.

Thanks ❤️

Edit: Removing the following lines from this PR

// packages/next/pages/_document.tsx
        let bytes = 0
        if (typeof Buffer !== 'undefined') {
          bytes = Buffer.from(data).byteLength
        } else {
          bytes = new TextEncoder().encode(data).length
        }

should make the error appear. I used the rsc demo app,

Edit2: Let me know if my explanation doesn't make sense.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
natew pushed a commit to natew/next.js that referenced this pull request Feb 16, 2022
…ercel#30642)

* if _app is not provided, fallback to default _app page
* If _document is not provided, fallback to inline functional components version or use the default
* if Document gIP is provided, error

Closes vercel#30654
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.

Unable to view localhost when "concurrentFeatures" is enabled. Requests are left to time out.
3 participants