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

Remove extra suspense boundary for default next/dynamic #64716

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

huozhi
Copy link
Member

@huozhi huozhi commented Apr 18, 2024

What

Removing the Suspense boundary on top of next/dynamic by default, make it as React.lazy component with preloading CSS feature.

Why

Extra Suspense boundary is causing extra useless rendering. For SSR, it shouldn't render loading by default

Related: #64060
Related: #64687
Closes NEXT-3074

This is sort of a breaking change, since removing the Suspense boundary on top of next/dynamic by default. If there's error happening in side the dynamic component you need to wrap an extra Suspense boundary on top of it

@ijjk ijjk added created-by: Next.js team PRs by the Next.js team. type: next labels Apr 18, 2024
@ijjk
Copy link
Member

ijjk commented Apr 18, 2024

Stats from current PR

Default Build (Increase detected ⚠️)
General Overall increase ⚠️
vercel/next.js canary vercel/next.js fix/remove-the-extra-suspense-boundary Change
buildDuration 25.7s 23.3s N/A
buildDurationCached 13.8s 13.2s N/A
nodeModulesSize 360 MB 360 MB ⚠️ +2.08 kB
nextStartRea..uration (ms) 642ms 661ms N/A
Client Bundles (main, webpack)
vercel/next.js canary vercel/next.js 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.09 kB 5.09 kB N/A
7953.HASH.js gzip 169 B 169 B
framework-HASH.js gzip 45.2 kB 45.2 kB
main-app-HASH.js gzip 231 B 230 B N/A
main-HASH.js gzip 31.5 kB 31.5 kB N/A
webpack-HASH.js gzip 1.65 kB 1.65 kB N/A
Overall change 45.4 kB 45.4 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary vercel/next.js 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 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.32 kB 4.32 kB N/A
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 4 kB 4 kB
Client Build Manifests
vercel/next.js canary vercel/next.js 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 fix/remove-the-extra-suspense-boundary Change
index.html gzip 528 B 526 B N/A
link.html gzip 541 B 539 B N/A
withRouter.html gzip 524 B 521 B N/A
Overall change 0 B 0 B
Edge SSR bundle Size
vercel/next.js canary vercel/next.js fix/remove-the-extra-suspense-boundary Change
edge-ssr.js gzip 94.7 kB 94.7 kB N/A
page.js gzip 181 kB 181 kB N/A
Overall change 0 B 0 B
Middleware size
vercel/next.js canary vercel/next.js fix/remove-the-extra-suspense-boundary Change
middleware-b..fest.js gzip 622 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 fix/remove-the-extra-suspense-boundary Change
app-page-exp...dev.js gzip 171 kB 171 kB
app-page-exp..prod.js gzip 98.5 kB 98.5 kB
app-page-tur..prod.js gzip 100 kB 100 kB
app-page-tur..prod.js gzip 94.3 kB 94.3 kB
app-page.run...dev.js gzip 157 kB 157 kB
app-page.run..prod.js gzip 93.1 kB 93.1 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 962 kB 962 kB
build cache Overall increase ⚠️
vercel/next.js canary vercel/next.js fix/remove-the-extra-suspense-boundary Change
0.pack gzip 1.62 MB 1.62 MB N/A
index.pack gzip 112 kB 113 kB ⚠️ +188 B
Overall change 112 kB 113 kB ⚠️ +188 B
Diff details
Diff for edge-ssr.js

Diff too large to display

Commit: ba2c9ba

@ijjk
Copy link
Member

ijjk commented Apr 18, 2024

Tests Passed

@snehakhobragade90
Copy link

@huozhi This fixes the issue I have reported last week next/dynamic renders loading state when React re-renders quickly client-side. My issue was particularly if using client-hooks like useEffect, but the end result of ‘triggering a react rerender quickly leading to a flicker of a loading state seems the same.

I tried copying these changes within the node_modules of our repo and it seems to have fixed the issue. By when can we expect this changeset to be released ?

@snehakhobragade90
Copy link

By when can we expect this changeset to land and be released, this fixes couple of issues #64687 and #64060

@huozhi huozhi force-pushed the fix/remove-the-extra-suspense-boundary branch from df7c9ad to ba2c9ba Compare April 29, 2024 21:56
@ijjk ijjk added the tests label Apr 29, 2024
@huozhi huozhi marked this pull request as ready for review April 30, 2024 14:08
@huozhi huozhi merged commit 029d1da into canary Apr 30, 2024
82 checks passed
@huozhi huozhi deleted the fix/remove-the-extra-suspense-boundary branch April 30, 2024 20:40
@joacub
Copy link

joacub commented May 2, 2024

this now wwhen you include something dynamic has flickering and going up to the page when dinamycs loads the content

ijjk added a commit that referenced this pull request May 3, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
…)"

This reverts commit 029d1da.
ijjk added a commit that referenced this pull request May 3, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
)

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

x-ref: [slack
thread](https://vercel.slack.com/archives/C04DUD7EB1B/p1714691721631339)

Reverts #64716

Closes NEXT-3307
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
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

4 participants