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

Revert "Remove extra suspense boundary for default next/dynamic" #65309

Merged
merged 1 commit into from
May 3, 2024

Conversation

ijjk
Copy link
Member

@ijjk ijjk commented May 3, 2024

Reverting temporarily while we investigate a bug where the page crashes due to the missing Suspense.

x-ref: slack thread

Reverts #64716

Closes NEXT-3307

@ijjk ijjk enabled auto-merge (squash) May 3, 2024 02:24
@ijjk
Copy link
Member Author

ijjk commented May 3, 2024

Stats from current PR

Default Build
General
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
buildDuration 15.8s 14.3s N/A
buildDurationCached 8s 7.6s N/A
nodeModulesSize 360 MB 360 MB N/A
nextStartRea..uration (ms) 393ms 394ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
1103-HASH.js gzip 31.9 kB 31.9 kB N/A
1a9f679d-HASH.js gzip 53.5 kB 53.5 kB N/A
335-HASH.js gzip 5.05 kB 5.05 kB
7953.HASH.js gzip 169 B 169 B
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 230 B 228 B N/A
main-HASH.js gzip 31.6 kB 31.6 kB N/A
webpack-HASH.js gzip 1.65 kB 1.65 kB N/A
Overall change 50.4 kB 50.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
_app-HASH.js gzip 192 B 193 B N/A
_error-HASH.js gzip 192 B 193 B N/A
amp-HASH.js gzip 507 B 511 B N/A
css-HASH.js gzip 341 B 343 B N/A
dynamic-HASH.js gzip 2.52 kB 2.52 kB
edge-ssr-HASH.js gzip 266 B 265 B N/A
head-HASH.js gzip 362 B 365 B N/A
hooks-HASH.js gzip 392 B 392 B
image-HASH.js gzip 4.27 kB 4.27 kB
index-HASH.js gzip 268 B 268 B
link-HASH.js gzip 2.69 kB 2.7 kB N/A
routerDirect..HASH.js gzip 329 B 328 B N/A
script-HASH.js gzip 397 B 397 B
withRouter-HASH.js gzip 324 B 324 B
1afbb74e6ecf..834.css gzip 106 B 106 B
Overall change 8.27 kB 8.27 kB
Client Build Manifests
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
_buildManifest.js gzip 483 B 484 B N/A
Overall change 0 B 0 B
Rendered Page Sizes
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
index.html gzip 528 B 528 B
link.html gzip 541 B 541 B
withRouter.html gzip 524 B 524 B
Overall change 1.59 kB 1.59 kB
Edge SSR bundle Size
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
edge-ssr.js gzip 94.7 kB 94.7 kB N/A
page.js gzip 182 kB 182 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
middleware-b..fest.js gzip 621 B 625 B N/A
middleware-r..fest.js gzip 156 B 156 B
middleware.js gzip 25.7 kB 25.7 kB N/A
edge-runtime..pack.js gzip 839 B 839 B
Overall change 995 B 995 B
Next Runtimes
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
app-page-exp...dev.js gzip 171 kB 171 kB
app-page-exp..prod.js gzip 98.7 kB 98.7 kB
app-page-tur..prod.js gzip 100 kB 100 kB
app-page-tur..prod.js gzip 94.5 kB 94.5 kB
app-page.run...dev.js gzip 157 kB 157 kB
app-page.run..prod.js gzip 93.2 kB 93.2 kB
app-route-ex...dev.js gzip 21.5 kB 21.5 kB
app-route-ex..prod.js gzip 15.2 kB 15.2 kB
app-route-tu..prod.js gzip 15.2 kB 15.2 kB
app-route-tu..prod.js gzip 15 kB 15 kB
app-route.ru...dev.js gzip 21.3 kB 21.3 kB
app-route.ru..prod.js gzip 15 kB 15 kB
pages-api-tu..prod.js gzip 9.55 kB 9.55 kB
pages-api.ru...dev.js gzip 9.82 kB 9.82 kB
pages-api.ru..prod.js gzip 9.55 kB 9.55 kB
pages-turbo...prod.js gzip 21.5 kB 21.5 kB
pages.runtim...dev.js gzip 22.1 kB 22.1 kB
pages.runtim..prod.js gzip 21.5 kB 21.5 kB
server.runti..prod.js gzip 51.6 kB 51.6 kB
Overall change 964 kB 964 kB
build cache
vercel/next.js canary vercel/next.js revert-64716-fix/remove-the-extra-suspense-boundary Change
0.pack gzip 1.62 MB 1.62 MB N/A
index.pack gzip 113 kB 113 kB N/A
Overall change 0 B 0 B
Diff details
Diff for edge-ssr.js

Diff too large to display

Commit: 865b7e2

@ijjk
Copy link
Member Author

ijjk commented May 3, 2024

Failing test suites

Commit: 865b7e2

pnpm test-start test/production/graceful-shutdown/index.test.ts

  • Graceful Shutdown > production (standalone mode) > should not accept new requests during shutdown cleanup > when there is no activity
Expand output

● Graceful Shutdown › production (standalone mode) › should not accept new requests during shutdown cleanup › when there is no activity

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

Expected: 0
Received: null

  228 |         // App finally shuts down
  229 |         await appKilledPromise
> 230 |         expect(app.exitCode).toBe(0)
      |                              ^
  231 |       })
  232 |     })
  233 |   }

  at Object.toBe (production/graceful-shutdown/index.test.ts:230:30)

Read more about building and testing Next.js in contributing.md.

@ijjk ijjk disabled auto-merge May 3, 2024 02:37
@ijjk ijjk merged commit 64ef34e into canary May 3, 2024
74 of 80 checks passed
@ijjk ijjk deleted the revert-64716-fix/remove-the-extra-suspense-boundary branch May 3, 2024 02:37
@snehakhobragade90
Copy link

@ijjk Do we have an ETA for the release of this changeset? The elimination of the Suspense boundary for dynamic/next imports addressed the issue documented on #64687. We conducted thorough testing internally with the canary build and found it to function as anticipated.
We are blocked to release our clients to PROD which is scheduled for early next week.
Also, we dont have visibility to the slack and issue posted on the revert PR, so missing the details on why we had to revert the change.

@huozhi
Copy link
Member

huozhi commented May 7, 2024

@snehakhobragade90 This is not the right fix for the issue, I commented here with a user-land solution. Let's move the discussion to the issue

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.

4 participants