Add initial `ResponsePayload` support #26938

devknoll · 2021-07-05T17:52:35Z

Refactors the internals of next-server to use ResponsePayload instead of string | null and manual sendPayload calls. This is the first step toward streaming support.

I split renderToResponseWithComponents into a separate renderToResponseWithComponentsInternal function for ease of review: GitHub's diff rendering was highly misleading, making it seem as though more of the function had changed. The separate function just makes the actual change clearer: we split renderToHTMLWithComponents into two promises; one that represents the actual render result, and one that represents all of the work (including background work for e.g. revalidation) that needs to be done as part of generating the result.

These changes make it easier to bubble up a ResponsePayload, instead of sometimes calling sendPayload out-of-band, centralizing all payload handling in sendResponse and eventually a similar function for public APIs that return a string. This centralization will make it much easier to handle a response that needs to be streamed, which is coming soon in another PR.

ijjk · 2021-07-05T18:21:33Z

Failing test suites

Commit: c94a597

test/acceptance/ReactRefreshLogBox.dev.test.js

server-side only compilation errors

Expand output

● server-side only compilation errors

ScriptTimeoutError: script timeout
  (Session info: headless chrome=91.0.4472.114)

  72 |
  73 |             // Wait for application to re-hydrate:
> 74 |             await browser.executeAsyncScript(function () {
     |             ^
  75 |               var callback = arguments[arguments.length - 1]
  76 |               if (window.__NEXT_HYDRATED) {
  77 |                 callback()

  at Object.throwDecodedError (../node_modules/selenium-webdriver/lib/error.js:550:15)
  at parseHttpResponse (../node_modules/selenium-webdriver/lib/http.js:565:13)
  at Executor.execute (../node_modules/selenium-webdriver/lib/http.js:491:26)
      at runMicrotasks (<anonymous>)
  at Proxy.execute (../node_modules/selenium-webdriver/lib/webdriver.js:700:17)
  at Object.patch (acceptance/helpers.js:74:13)
  at Object.<anonymous> (acceptance/ReactRefreshLogBox.dev.test.js:1415:3)

devknoll · 2021-07-05T18:46:34Z

Failing test seems unrelated and sporadic. It passed on our fork.

ijjk · 2021-07-06T03:16:47Z

Stats from current PR

Default Build (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
buildDuration	12.2s	12.5s	⚠️ +363ms
buildDurationCached	3s	3.2s	⚠️ +156ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +4.09 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.131	2.24	⚠️ +0.11
/ avg req/sec	1173.03	1115.9	⚠️ -57.13
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.183	1.258	⚠️ +0.07
/error-in-render avg req/sec	2112.86	1987.59	⚠️ -125.27

Client Bundles (main, webpack, commons)

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
359.HASH.js gzip	3.09 kB	3.09 kB	✓
framework-HASH.js gzip	42 kB	42 kB	✓
main-HASH.js gzip	20.6 kB	20.6 kB	✓
webpack-HASH.js gzip	1.49 kB	1.49 kB	✓
Overall change	67.2 kB	67.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
polyfills-HASH.js gzip	31.1 kB	31.1 kB	✓
Overall change	31.1 kB	31.1 kB	✓

Client Pages

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
_app-HASH.js gzip	803 B	803 B	✓
_error-HASH.js gzip	3.18 kB	3.18 kB	✓
amp-HASH.js gzip	526 B	526 B	✓
css-HASH.js gzip	329 B	329 B	✓
hooks-HASH.js gzip	903 B	903 B	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	1.65 kB	1.65 kB	✓
routerDirect..HASH.js gzip	322 B	322 B	✓
withRouter-HASH.js gzip	320 B	320 B	✓
bb14e60e810b..30f.css gzip	125 B	125 B	✓
Overall change	8.42 kB	8.42 kB	✓

Client Build Manifests

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
_buildManifest.js gzip	390 B	390 B	✓
Overall change	390 B	390 B	✓

Rendered Page Sizes

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
index.html gzip	523 B	523 B	✓
link.html gzip	537 B	537 B	✓
withRouter.html gzip	515 B	515 B	✓
Overall change	1.57 kB	1.57 kB	✓

Webpack 4 Mode (Decrease detected ✓)

General Overall increase ⚠️

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
buildDuration	11.7s	11.6s	-178ms
buildDurationCached	4.7s	4.7s	-39ms
nodeModulesSize	49.3 MB	49.3 MB	⚠️ +4.09 kB

Page Load Tests Overall decrease ⚠️

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
/ failed reqs	0	0	✓
/ total time (seconds)	2.295	2.367	⚠️ +0.07
/ avg req/sec	1089.37	1056.4	⚠️ -32.97
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.25	1.333	⚠️ +0.08
/error-in-render avg req/sec	1999.86	1875.19	⚠️ -124.67

Client Bundles (main, webpack, commons)

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
14.HASH.js gzip	3.11 kB	3.11 kB	✓
677f882d2ed8..HASH.js gzip	13.9 kB	13.9 kB	✓
framework.HASH.js gzip	41.8 kB	41.8 kB	✓
main-HASH.js gzip	7.81 kB	7.81 kB	✓
webpack-HASH.js gzip	1.19 kB	1.19 kB	✓
Overall change	67.8 kB	67.8 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
polyfills-HASH.js gzip	31.3 kB	31.3 kB	✓
Overall change	31.3 kB	31.3 kB	✓

Client Pages

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
_app-HASH.js gzip	791 B	791 B	✓
_error-HASH.js gzip	3.83 kB	3.83 kB	✓
amp-HASH.js gzip	531 B	531 B	✓
css-HASH.js gzip	333 B	333 B	✓
hooks-HASH.js gzip	910 B	910 B	✓
index-HASH.js gzip	227 B	227 B	✓
link-HASH.js gzip	1.64 kB	1.64 kB	✓
routerDirect..HASH.js gzip	295 B	295 B	✓
withRouter-HASH.js gzip	292 B	292 B	✓
e025d2764813..52f.css gzip	125 B	125 B	✓
Overall change	8.98 kB	8.98 kB	✓

Client Build Manifests

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
_buildManifest.js gzip	418 B	418 B	✓
Overall change	418 B	418 B	✓

Rendered Page Sizes

	vercel/next.js canary	azukaru/next.js x-streaming-simplified	Change
index.html gzip	568 B	568 B	✓
link.html gzip	581 B	581 B	✓
withRouter.html gzip	561 B	561 B	✓
Overall change	1.71 kB	1.71 kB	✓

Commit: feca948

Refactors the internals of `next-server` to use `ResponsePayload` instead of `string | null` and manual `sendPayload` calls. This is the first step toward streaming support. I split `renderToResponseWithComponents` into a separate `renderToResponseWithComponentsInternal` function for ease of review: GitHub's diff rendering was highly misleading, making it seem as though more of the function had changed. The separate function just makes the actual change clearer: we split `renderToHTMLWithComponents` into two promises; one that represents the actual render result, and one that represents all of the work (including background work for e.g. revalidation) that needs to be done as part of generating the result. These changes make it easier to bubble up a `ResponsePayload`, instead of sometimes calling `sendPayload` out-of-band, centralizing all payload handling in `sendResponse` and eventually a similar function for public APIs that return a string. This centralization will make it much easier to handle a response that needs to be streamed, which is coming soon in another PR.

devknoll added 12 commits July 4, 2021 16:08

Add initial RenderResult

d804820

Finalize initial RenderResult

4dbced8

Remove unnecessary async

6e01e42

Fix tests

f36d70f

Misc

bd20d46

Revert removal of sendResult from dev server for now

166d92b

Merge branch 'canary' into x-streaming-simplified

9c0c052

renderToHTMLWithComponents -> renderToResultWithComponents

8612f7e

Clean up GitHub diff

6dc4309

Fix test

e50a6b4

Fix test

68048c8

Merge branch 'canary' into x-streaming-simplified

c94a597

ijjk added created-by: Chrome Aurora type: next labels Jul 5, 2021

This comment has been minimized.

Sign in to view

devknoll marked this pull request as ready for review July 5, 2021 18:15

devknoll requested review from huozhi, ijjk, padmaia, shuding, styfle and timneutkens as code owners July 5, 2021 18:15

Rename RenderResult -> ResponsePayload

787a8d6

devknoll changed the title ~~Add initial RenderResult support~~ Add initial ResponsePayload support Jul 6, 2021

Fix broken import

feca948

timneutkens approved these changes Jul 6, 2021

View reviewed changes

kodiakhq bot merged commit 2795b79 into vercel:canary Jul 6, 2021

vercel locked as resolved and limited conversation to collaborators Jan 28, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add initial `ResponsePayload` support #26938

Add initial `ResponsePayload` support #26938

devknoll commented Jul 5, 2021 •

edited

Loading

This comment has been minimized.

ijjk commented Jul 5, 2021

devknoll commented Jul 5, 2021

ijjk commented Jul 6, 2021

Add initial ResponsePayload support #26938

Add initial ResponsePayload support #26938

Conversation

devknoll commented Jul 5, 2021 • edited Loading

This comment has been minimized.

ijjk commented Jul 5, 2021

Failing test suites

devknoll commented Jul 5, 2021

ijjk commented Jul 6, 2021

Stats from current PR

Add initial `ResponsePayload` support #26938

Add initial `ResponsePayload` support #26938

devknoll commented Jul 5, 2021 •

edited

Loading