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

Merge middleware manifest generated in different compilers #32442

Closed
wants to merge 7 commits into from

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Dec 13, 2021

Related: #32504

  • Merge the manifest generated in client compiler and web runtime compiler
  • Add test for middleware manifest when hash of static/chunks/webpack-middleware is inconsistent
  • remove the unminified version runtime chunk webpack-middleware
  • the read and write jobs in build phase doesn't share the reading manifest utils since it's async with fs and require might not be a good idea there

@ijjk ijjk added the created-by: Next.js team PRs by the Next.js team. label Dec 13, 2021
@ijjk

This comment has been minimized.

@ijjk

This comment has been minimized.

@huozhi huozhi changed the title Add test for middleware manifest + rsc Merge middleware manifest generated in different compilers Dec 13, 2021
@huozhi huozhi force-pushed the middleware-manifest-hash branch from 8c64495 to 9a8602b Compare December 13, 2021 20:41
@ijjk

This comment has been minimized.

@ijjk
Copy link
Member

ijjk commented Dec 13, 2021

Failing test suites

Commit: 9a8602b

test/integration/middleware/core/test/index.test.js

  • Middleware base tests > production mode > should have correct files in manifest
Expand output

● Middleware base tests › production mode › should have correct files in manifest

expect(received).toBe(expected) // Object.is equality

Expected: true
Received: false

  83 |     })
  84 |
> 85 |     it('should have middleware warning during start', () => {
     |                    ^
  86 |       expect(serverOutput).toContain(middlewareWarning)
  87 |     })
  88 |

  at Object.<anonymous> (integration/middleware/core/test/index.test.js:85:20)

@ijjk

This comment has been minimized.

@huozhi huozhi marked this pull request as ready for review December 13, 2021 22:17
Copy link
Member

@shuding shuding left a comment

Choose a reason for hiding this comment

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

LGTM as a temporary solution, cc @sokra for the terser-webpack-plugin related changes.

@huozhi
Copy link
Member Author

huozhi commented Dec 14, 2021

Most of changes are added in #30151, wonder if it's okay to remove emitting the unminified version of runtime chunk now? cc @ijjk

Copy link
Member

@ijjk ijjk left a comment

Choose a reason for hiding this comment

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

Looks like this is causing the intermediate value error again on the middleware on nextjs.org on a test deployment with these changes

[TypeError: (intermediate value)(intermediate value)(intermediate value)(...) is not a function]

@huozhi huozhi marked this pull request as draft December 18, 2021 12:07
@huozhi huozhi linked an issue Dec 22, 2021 that may be closed by this pull request
@ijjk
Copy link
Member

ijjk commented Dec 24, 2021

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
buildDuration 17.7s 18.1s ⚠️ +368ms
buildDurationCached 4.2s 4s -209ms
nodeModulesSize 348 MB 348 MB ⚠️ +2.89 kB
Page Load Tests Overall increase ✓
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
/ failed reqs 0 0
/ total time (seconds) 3.803 3.936 ⚠️ +0.13
/ avg req/sec 657.44 635.21 ⚠️ -22.23
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 2.213 1.987 -0.23
/error-in-render avg req/sec 1129.44 1258.02 +128.58
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
450.HASH.js gzip 179 B 179 B
framework-HASH.js gzip 42.2 kB 42.2 kB
main-HASH.js gzip 27 kB 27 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 huozhi/next.js middleware-manifest-hash Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary huozhi/next.js middleware-manifest-hash 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.73 kB 4.73 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 huozhi/next.js middleware-manifest-hash Change
_buildManifest.js gzip 459 B 459 B
Overall change 459 B 459 B
Rendered Page Sizes
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
index.html gzip 533 B 533 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 (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
buildDuration 20.2s 20.7s ⚠️ +505ms
buildDurationCached 4s 4s ⚠️ +11ms
nodeModulesSize 348 MB 348 MB ⚠️ +2.89 kB
Page Load Tests Overall increase ✓
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
/ failed reqs 0 0
/ total time (seconds) 3.788 3.786 0
/ avg req/sec 659.95 660.41 +0.46
/error-in-render failed reqs 0 0
/error-in-render total time (seconds) 2.127 2.083 -0.04
/error-in-render avg req/sec 1175.09 1200.06 +24.97
Client Bundles (main, webpack, commons)
vercel/next.js canary huozhi/next.js middleware-manifest-hash 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 huozhi/next.js middleware-manifest-hash Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary huozhi/next.js middleware-manifest-hash 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.75 kB 4.75 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 huozhi/next.js middleware-manifest-hash Change
_buildManifest.js gzip 458 B 458 B
Overall change 458 B 458 B
Rendered Page Sizes
vercel/next.js canary huozhi/next.js middleware-manifest-hash Change
index.html gzip 532 B 532 B
link.html gzip 545 B 545 B
withRouter.html gzip 525 B 525 B
Overall change 1.6 kB 1.6 kB
Commit: 12df321

@huozhi huozhi closed this Feb 3, 2022
@huozhi huozhi deleted the middleware-manifest-hash branch February 3, 2022 15:14
@huozhi huozhi restored the middleware-manifest-hash branch February 3, 2022 15:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
3 participants