Use `react-dom/server.browser` in Node.js #33950

devknoll · 2022-02-03T18:47:46Z

Instead of branching rendering based on Node.js and browser/web runtimes, we should just use the web version for now, which can run as-is on versions >=16.5.0 of Node.js, polyfilling ReadableStream on older versions when necessary.

There are a few potential downsides to this, as React is less able to optimize flushing and execution. We can revisit that in the future though if desired.

ijjk · 2022-02-04T06:05:19Z

Failing test suites

Commit: b45c6a5

test/integration/react-streaming-and-server-components/test/index.test.js

concurrentFeatures - basic > should handle suspense error page correctly (node stream)
concurrentFeatures - dev > should render 404 error correctly
concurrentFeatures - prod > should render 404 error correctly

Expand output

● concurrentFeatures - basic › should handle suspense error page correctly (node stream)

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

Expected: "custom-404-pagenext_streaming_data"
Received: null

  151 |       `document.querySelector('#__next').textContent`
  152 |     )
> 153 |     expect(hydrationContent).toBe('custom-404-pagenext_streaming_data')
      |                              ^
  154 |
  155 |     error404Page.restore()
  156 |   })

  at Object.<anonymous> (integration/react-streaming-and-server-components/test/index.test.js:153:30)

● concurrentFeatures - prod › should render 404 error correctly

expect(received).toContain(expected) // indexOf

Expected substring: "custom-404-page"
Received string:    "Internal Server Error"

  340 |     const pathNotFoundHTML = await renderViaHTTP(context.appPort, '/not-found')
  341 |
> 342 |     expect(path404HTML).toContain('custom-404-page')
      |                         ^
  343 |     expect(pathNotFoundHTML).toContain('custom-404-page')
  344 |   })
  345 |

  at Object.<anonymous> (integration/react-streaming-and-server-components/test/index.test.js:342:25)

● concurrentFeatures - dev › should render 404 error correctly

thrown: "Exceeded timeout of 90000 ms for a test.
Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

  336 |   })
  337 |
> 338 |   it('should render 404 error correctly', async () => {
      |   ^
  339 |     const path404HTML = await renderViaHTTP(context.appPort, '/404')
  340 |     const pathNotFoundHTML = await renderViaHTTP(context.appPort, '/not-found')
  341 |

  at runBasicTests (integration/react-streaming-and-server-components/test/index.test.js:338:3)
  at integration/react-streaming-and-server-components/test/index.test.js:298:3
  at Object.<anonymous> (integration/react-streaming-and-server-components/test/index.test.js:264:1)

ijjk · 2022-02-04T17:39:12Z

Stats from current PR

Default Build (Increase detected ⚠️)

General Overall decrease ✓

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
buildDuration	17.1s	17.2s	⚠️ +103ms
buildDurationCached	6.6s	6.6s	⚠️ +35ms
nodeModulesSize	359 MB	359 MB	-4.86 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.284	2.993	-0.29
/ avg req/sec	761.18	835.32	+74.14
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.493	1.506	⚠️ +0.01
/error-in-render avg req/sec	1675.04	1659.77	⚠️ -15.27

Client Bundles (main, webpack, commons)

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.2 kB	42.2 kB	✓
main-HASH.js gzip	27.3 kB	27.3 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.2 kB	71.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	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.94 kB	4.94 kB	✓
index-HASH.js gzip	263 B	263 B	✓
link-HASH.js gzip	2.19 kB	2.19 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.3 kB	14.3 kB	✓

Client Build Manifests

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
index.html gzip	528 B	528 B	✓
link.html gzip	543 B	543 B	✓
withRouter.html gzip	524 B	524 B	✓
Overall change	1.59 kB	1.59 kB	✓

Default Build with SWC (Increase detected ⚠️)

General Overall decrease ✓

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
buildDuration	20.7s	20.7s	⚠️ +48ms
buildDurationCached	6.7s	6.9s	⚠️ +194ms
nodeModulesSize	359 MB	359 MB	-4.86 kB

Page Load Tests Overall increase ✓

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
/ failed reqs	0	0	✓
/ total time (seconds)	3.316	2.994	-0.32
/ avg req/sec	753.98	834.98	+81
/error-in-render failed reqs	0	0	✓
/error-in-render total time (seconds)	1.472	1.518	⚠️ +0.05
/error-in-render avg req/sec	1698.39	1647.27	⚠️ -51.12

Client Bundles (main, webpack, commons)

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
450.HASH.js gzip	179 B	179 B	✓
framework-HASH.js gzip	42.3 kB	42.3 kB	✓
main-HASH.js gzip	27.4 kB	27.4 kB	✓
webpack-HASH.js gzip	1.44 kB	1.44 kB	✓
Overall change	71.3 kB	71.3 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
polyfills-HASH.js gzip	31 kB	31 kB	✓
Overall change	31 kB	31 kB	✓

Client Pages

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	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	911 B	911 B	✓
image-HASH.js gzip	4.97 kB	4.97 kB	✓
index-HASH.js gzip	256 B	256 B	✓
link-HASH.js gzip	2.21 kB	2.21 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.3 kB	14.3 kB	✓

Client Build Manifests

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
_buildManifest.js gzip	459 B	459 B	✓
Overall change	459 B	459 B	✓

Rendered Page Sizes

	vercel/next.js canary	azukaru/next.js x-rm-node-stream	Change
index.html gzip	532 B	532 B	✓
link.html gzip	545 B	545 B	✓
withRouter.html gzip	526 B	526 B	✓
Overall change	1.6 kB	1.6 kB	✓

Commit: f7e3c7f

Instead of branching rendering based on Node.js and browser/web runtimes, we should just use the web version for now, which can run as-is on versions >=16.5.0 of Node.js, polyfilling `ReadableStream` on older versions when necessary. There are a few potential downsides to this, as React is less able to optimize flushing and execution. We can revisit that in the future though if desired.

Remove Node.js stream rendering

d7cb919

ijjk added created-by: Chrome Aurora PRs by the Google Chrome team: https://web.dev/aurora type: next labels Feb 3, 2022

This comment has been minimized.

Sign in to view

devknoll added 2 commits February 3, 2022 18:56

Support for destroy

b9617ad

Fix polyfill pathname

16df13f

devknoll changed the title ~~Remove Node.js stream rendering~~ Use ReadableStream in Node.js Feb 3, 2022

devknoll added 2 commits February 3, 2022 19:06

Fix lint

44e769b

Add cork/uncork stubs

ed1bb1b

This comment has been minimized.

Sign in to view

devknoll added 2 commits February 3, 2022 19:35

ES import of polyfill

22309a8

Merge branch 'canary' into x-rm-node-stream

723add1

This comment has been minimized.

Sign in to view

Isolate ReadableStream polyfill to fix tests

9835b58

This comment has been minimized.

Sign in to view

devknoll added 2 commits February 4, 2022 05:30

Fix module resolution

b45c6a5

Fix lint

c0b28c5

This comment has been minimized.

Sign in to view

devknoll added 2 commits February 4, 2022 06:12

Fix module

8ed84c0

Merge branch 'canary' into x-rm-node-stream

7d52562

This comment has been minimized.

Sign in to view

devknoll added 2 commits February 4, 2022 16:11

Merge branch 'canary' into x-rm-node-stream

1e81815

NodeWritablePiper -> ResultPiper

d1aa65e

devknoll changed the title ~~Use ReadableStream in Node.js~~ Use react-dom/server.browser in Node.js Feb 4, 2022

This comment has been minimized.

Sign in to view

devknoll marked this pull request as ready for review February 4, 2022 17:07

devknoll requested a review from huozhi as a code owner February 4, 2022 17:07

devknoll requested review from ijjk, padmaia, shuding and timneutkens as code owners February 4, 2022 17:07

shuding approved these changes Feb 4, 2022

View reviewed changes

devknoll added 2 commits February 4, 2022 17:15

Merge branch 'canary' into x-rm-node-stream

8ae16eb

Merge branch 'canary' into x-rm-node-stream

f7e3c7f

huozhi approved these changes Feb 4, 2022

View reviewed changes

ijjk approved these changes Feb 4, 2022

View reviewed changes

kodiakhq bot merged commit 0b1d5e1 into vercel:canary Feb 4, 2022

github-actions bot locked as resolved and limited conversation to collaborators Mar 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `react-dom/server.browser` in Node.js #33950

Use `react-dom/server.browser` in Node.js #33950

devknoll commented Feb 3, 2022 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 4, 2022

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 4, 2022

Use react-dom/server.browser in Node.js #33950

Use react-dom/server.browser in Node.js #33950

Conversation

devknoll commented Feb 3, 2022 • edited Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 4, 2022

Failing test suites

This comment has been minimized.

This comment has been minimized.

ijjk commented Feb 4, 2022

Stats from current PR

Use `react-dom/server.browser` in Node.js #33950

Use `react-dom/server.browser` in Node.js #33950

devknoll commented Feb 3, 2022 •

edited

Loading