-
Notifications
You must be signed in to change notification settings - Fork 27.8k
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
[use-cache] Exclude warmup render when dynamicIO is off #75527
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix makes sense to me. A test would be nice though, as I wasn't able to reproduce the mentioned issue easily.
f2827aa
to
8f0e25d
Compare
Was tricky to repro. Needs to have a loading.js to get the wamrup to do anything useful and then need to have dangling promises that are not already handled like the built-in ones are |
Now that this is clarified we can delete this TODO comment: next.js/packages/next/src/server/app-render/app-render.tsx Lines 684 to 687 in 58e1bd2
|
e4b282e
to
95901b1
Compare
Failing test suitesCommit: 95901b1
Expand output● rewrite-headers › middleware rewrite external RSC (/hello/vercel) › should have the expected headers
● rewrite-headers › middleware rewrite external Prefetch RSC (/hello/vercel) › should have the expected headers
Read more about building and testing Next.js in contributing.md. |
when using use-cache standalone we don't need the dev warmup render becuase it is for addressing environment names of Prerender in DIO mode. This fixes a bug where rejected promises from cookies appeared in server logs when use cache was enabled without dynamicIO.
95901b1
to
29a0485
Compare
Stats from current PRDefault Build (Increase detected
|
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
buildDuration | 19.4s | 17.7s | N/A |
buildDurationCached | 16.8s | 14.3s | N/A |
nodeModulesSize | 391 MB | 391 MB | N/A |
nextStartRea..uration (ms) | 464ms | 446ms | N/A |
Client Bundles (main, webpack)
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
5306-HASH.js gzip | 54 kB | 53.9 kB | N/A |
8276.HASH.js gzip | 169 B | 168 B | N/A |
8377-HASH.js gzip | 5.46 kB | 5.46 kB | N/A |
bccd1874-HASH.js gzip | 52.9 kB | 52.9 kB | N/A |
framework-HASH.js gzip | 57.5 kB | 57.5 kB | N/A |
main-app-HASH.js gzip | 240 B | 242 B | N/A |
main-HASH.js gzip | 34.5 kB | 34.4 kB | N/A |
webpack-HASH.js gzip | 1.71 kB | 1.71 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Legacy Client Bundles (polyfills)
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
polyfills-HASH.js gzip | 39.4 kB | 39.4 kB | ✓ |
Overall change | 39.4 kB | 39.4 kB | ✓ |
Client Pages
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
_app-HASH.js gzip | 193 B | 193 B | ✓ |
_error-HASH.js gzip | 193 B | 193 B | ✓ |
amp-HASH.js gzip | 512 B | 510 B | N/A |
css-HASH.js gzip | 343 B | 342 B | N/A |
dynamic-HASH.js gzip | 1.84 kB | 1.84 kB | ✓ |
edge-ssr-HASH.js gzip | 265 B | 265 B | ✓ |
head-HASH.js gzip | 363 B | 362 B | N/A |
hooks-HASH.js gzip | 393 B | 392 B | N/A |
image-HASH.js gzip | 4.59 kB | 4.58 kB | N/A |
index-HASH.js gzip | 268 B | 268 B | ✓ |
link-HASH.js gzip | 2.35 kB | 2.35 kB | N/A |
routerDirect..HASH.js gzip | 328 B | 328 B | ✓ |
script-HASH.js gzip | 397 B | 397 B | ✓ |
withRouter-HASH.js gzip | 323 B | 326 B | N/A |
1afbb74e6ecf..834.css gzip | 106 B | 106 B | ✓ |
Overall change | 3.59 kB | 3.59 kB | ✓ |
Client Build Manifests
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
_buildManifest.js gzip | 748 B | 747 B | N/A |
Overall change | 0 B | 0 B | ✓ |
Rendered Page Sizes
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
index.html gzip | 523 B | 524 B | N/A |
link.html gzip | 538 B | 538 B | ✓ |
withRouter.html gzip | 519 B | 520 B | N/A |
Overall change | 538 B | 538 B | ✓ |
Edge SSR bundle Size
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
edge-ssr.js gzip | 129 kB | 129 kB | N/A |
page.js gzip | 210 kB | 210 kB | N/A |
Overall change | 0 B | 0 B | ✓ |
Middleware size
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
middleware-b..fest.js gzip | 669 B | 667 B | N/A |
middleware-r..fest.js gzip | 155 B | 156 B | N/A |
middleware.js gzip | 31.3 kB | 31.3 kB | N/A |
edge-runtime..pack.js gzip | 844 B | 844 B | ✓ |
Overall change | 844 B | 844 B | ✓ |
Next Runtimes
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
app-page-exp...dev.js gzip | 386 kB | 386 kB | ✓ |
app-page-exp..prod.js gzip | 132 kB | 132 kB | ✓ |
app-page-tur..prod.js gzip | 145 kB | 145 kB | ✓ |
app-page-tur..prod.js gzip | 141 kB | 141 kB | ✓ |
app-page.run...dev.js gzip | 373 kB | 373 kB | N/A |
app-page.run..prod.js gzip | 128 kB | 128 kB | ✓ |
app-route-ex...dev.js gzip | 39.2 kB | 39.2 kB | ✓ |
app-route-ex..prod.js gzip | 25.6 kB | 25.6 kB | ✓ |
app-route-tu..prod.js gzip | 25.6 kB | 25.6 kB | ✓ |
app-route-tu..prod.js gzip | 25.4 kB | 25.4 kB | ✓ |
app-route.ru...dev.js gzip | 40.8 kB | 40.8 kB | ✓ |
app-route.ru..prod.js gzip | 25.4 kB | 25.4 kB | ✓ |
dist_client_...dev.js gzip | 356 B | 356 B | ✓ |
dist_client_...dev.js gzip | 349 B | 349 B | ✓ |
pages-api-tu..prod.js gzip | 9.69 kB | 9.69 kB | ✓ |
pages-api.ru...dev.js gzip | 11.8 kB | 11.8 kB | ✓ |
pages-api.ru..prod.js gzip | 9.68 kB | 9.68 kB | ✓ |
pages-turbo...prod.js gzip | 21.9 kB | 21.9 kB | ✓ |
pages.runtim...dev.js gzip | 31.5 kB | 31.5 kB | ✓ |
pages.runtim..prod.js gzip | 21.9 kB | 21.9 kB | ✓ |
server.runti..prod.js gzip | 60.2 kB | 60.2 kB | N/A |
Overall change | 1.22 MB | 1.22 MB | ✓ |
build cache Overall increase ⚠️
vercel/next.js canary | gnoff/next.js use-cache-exclude-warmup | Change | |
---|---|---|---|
0.pack gzip | 2.1 MB | 2.1 MB | |
index.pack gzip | 75.3 kB | 75.4 kB | |
Overall change | 2.18 MB | 2.18 MB |
Diff details
Diff for edge-ssr.js
Diff too large to display
Diff for main-HASH.js
Diff too large to display
Diff for server.runtime.prod.js
Diff too large to display
when using use-cache standalone we don't need the dev warmup render becuase it is for addressing environment names of Prerender in DIO mode. This fixes a bug where rejected promises from cookies appeared in server logs when use cache was enabled without dynamicIO.