From 7a0374092f9ff3ba43288b7be7422cf91fd542aa Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 3 May 2022 23:03:11 +0200 Subject: [PATCH 1/3] fix: react root enabled properly in custom server --- packages/next/bin/next.ts | 5 +---- packages/next/server/next.ts | 6 ++++++ packages/next/shared/lib/react.ts | 5 +++++ test/integration/custom-server/test/index.test.js | 10 ++++++++++ 4 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 packages/next/shared/lib/react.ts diff --git a/packages/next/bin/next.ts b/packages/next/bin/next.ts index bb99c8ad64719..6f26cb1c97ecb 100755 --- a/packages/next/bin/next.ts +++ b/packages/next/bin/next.ts @@ -2,6 +2,7 @@ import * as log from '../build/output/log' import arg from 'next/dist/compiled/arg/index.js' import { NON_STANDARD_NODE_ENV } from '../lib/constants' +import { shouldUseReactRoot } from '../shared/lib/react' ;['react', 'react-dom'].forEach((dependency) => { try { // When 'npm link' is used it checks the clone location. Not the project. @@ -42,10 +43,6 @@ const args = arg( } ) -// Detect if react-dom is enabled streaming rendering mode -const shouldUseReactRoot = !!require('react-dom/server.browser') - .renderToReadableStream - // Version is inlined into the file using taskr build pipeline if (args['--version']) { console.log(`Next.js v${process.env.__NEXT_VERSION}`) diff --git a/packages/next/server/next.ts b/packages/next/server/next.ts index 31f6f75024385..b774826da1eda 100644 --- a/packages/next/server/next.ts +++ b/packages/next/server/next.ts @@ -12,6 +12,12 @@ import { PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants' import { PHASE_PRODUCTION_SERVER } from '../shared/lib/constants' import { IncomingMessage, ServerResponse } from 'http' import { NextUrlWithParsedQuery } from './request-meta' +import { shouldUseReactRoot } from '../shared/lib/react' + +// Make sure env of custom server is overridden +if (shouldUseReactRoot) { + ;(process.env as any).__NEXT_REACT_ROOT = 'true' +} let ServerImpl: typeof Server diff --git a/packages/next/shared/lib/react.ts b/packages/next/shared/lib/react.ts new file mode 100644 index 0000000000000..f58ab7c261d11 --- /dev/null +++ b/packages/next/shared/lib/react.ts @@ -0,0 +1,5 @@ +// @ts-ignore +import ReactDOMServerBrowser from 'react-dom/server.browser' + +// Detect if react-dom is enabled streaming rendering mode +export const shouldUseReactRoot = !!ReactDOMServerBrowser.renderToReadableStream diff --git a/test/integration/custom-server/test/index.test.js b/test/integration/custom-server/test/index.test.js index 330fe4b1c4ef5..8d4e2ee70311e 100644 --- a/test/integration/custom-server/test/index.test.js +++ b/test/integration/custom-server/test/index.test.js @@ -132,8 +132,18 @@ describe('Custom Server', () => { try { browser = await webdriver(context.appPort, '/test-index-hmr') const text = await browser.elementByCss('#go-asset').text() + const logs = await browser.log() expect(text).toBe('Asset') + // Hydrates with react 18 is correct as expected + expect( + logs.some((log) => + log.message.includes( + 'ReactDOM.hydrate is no longer supported in React 18' + ) + ) + ).toBe(false) + indexPg.replace('Asset', 'Asset!!') await check(() => browser.elementByCss('#go-asset').text(), /Asset!!/) From ec8e3a0e30ae0a005cc46f9c6c4a4bd6f3c82de6 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Tue, 3 May 2022 23:39:24 +0200 Subject: [PATCH 2/3] hoist react version, dynamic require react dom --- packages/next/bin/next.ts | 5 ++++- packages/next/client/index.tsx | 8 +++++--- packages/next/server/next.ts | 18 ++++++++++++------ packages/next/shared/lib/react.ts | 5 ----- 4 files changed, 21 insertions(+), 15 deletions(-) delete mode 100644 packages/next/shared/lib/react.ts diff --git a/packages/next/bin/next.ts b/packages/next/bin/next.ts index 6f26cb1c97ecb..bb99c8ad64719 100755 --- a/packages/next/bin/next.ts +++ b/packages/next/bin/next.ts @@ -2,7 +2,6 @@ import * as log from '../build/output/log' import arg from 'next/dist/compiled/arg/index.js' import { NON_STANDARD_NODE_ENV } from '../lib/constants' -import { shouldUseReactRoot } from '../shared/lib/react' ;['react', 'react-dom'].forEach((dependency) => { try { // When 'npm link' is used it checks the clone location. Not the project. @@ -43,6 +42,10 @@ const args = arg( } ) +// Detect if react-dom is enabled streaming rendering mode +const shouldUseReactRoot = !!require('react-dom/server.browser') + .renderToReadableStream + // Version is inlined into the file using taskr build pipeline if (args['--version']) { console.log(`Next.js v${process.env.__NEXT_VERSION}`) diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index c4ec6dfe7aa51..f966b50224db3 100644 --- a/packages/next/client/index.tsx +++ b/packages/next/client/index.tsx @@ -1,7 +1,6 @@ /* global location */ import '../build/polyfills/polyfill-module' import React, { useState } from 'react' -import ReactDOM from 'react-dom' import { HeadManagerContext } from '../shared/lib/head-manager-context' import mitt, { MittEmitter } from '../shared/lib/mitt' import { RouterContext } from '../shared/lib/router-context' @@ -41,6 +40,10 @@ import { RefreshContext } from './streaming/refresh' import { ImageConfigContext } from '../shared/lib/image-config-context' import { ImageConfigComplete } from '../shared/lib/image-config' +const ReactDOM = process.env.__NEXT_REACT_ROOT + ? require('react-dom/client') + : require('react-dom') + /// declare let __webpack_public_path__: string @@ -549,8 +552,7 @@ function renderReactElement( if (process.env.__NEXT_REACT_ROOT) { if (!reactRoot) { // Unlike with createRoot, you don't need a separate root.render() call here - const ReactDOMClient = require('react-dom/client') - reactRoot = ReactDOMClient.hydrateRoot(domEl, reactEl) + reactRoot = ReactDOM.hydrateRoot(domEl, reactEl) // TODO: Remove shouldHydrate variable when React 18 is stable as it can depend on `reactRoot` existing shouldHydrate = false } else { diff --git a/packages/next/server/next.ts b/packages/next/server/next.ts index b774826da1eda..7e7e787d7f9b2 100644 --- a/packages/next/server/next.ts +++ b/packages/next/server/next.ts @@ -12,12 +12,6 @@ import { PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants' import { PHASE_PRODUCTION_SERVER } from '../shared/lib/constants' import { IncomingMessage, ServerResponse } from 'http' import { NextUrlWithParsedQuery } from './request-meta' -import { shouldUseReactRoot } from '../shared/lib/react' - -// Make sure env of custom server is overridden -if (shouldUseReactRoot) { - ;(process.env as any).__NEXT_REACT_ROOT = 'true' -} let ServerImpl: typeof Server @@ -42,6 +36,7 @@ export class NextServer { private server?: Server private reqHandlerPromise?: Promise private preparedAssetPrefix?: string + private reactRootEnabled?: boolean public options: NextServerOptions constructor(options: NextServerOptions) { @@ -57,6 +52,17 @@ export class NextServer { } getRequestHandler(): RequestHandler { + // Make sure env of custom server is overridden + if (this.reactRootEnabled === undefined) { + // Use dynamic require to make sure it's executed in it's own context + const ReactDOMServer = require('react-dom/server.browser') + this.reactRootEnabled = !!ReactDOMServer.renderToReadableStream + + if (this.reactRootEnabled) { + ;(process.env as any).__NEXT_REACT_ROOT = 'true' + } + } + return async ( req: IncomingMessage, res: ServerResponse, diff --git a/packages/next/shared/lib/react.ts b/packages/next/shared/lib/react.ts deleted file mode 100644 index f58ab7c261d11..0000000000000 --- a/packages/next/shared/lib/react.ts +++ /dev/null @@ -1,5 +0,0 @@ -// @ts-ignore -import ReactDOMServerBrowser from 'react-dom/server.browser' - -// Detect if react-dom is enabled streaming rendering mode -export const shouldUseReactRoot = !!ReactDOMServerBrowser.renderToReadableStream From 621acc2d0f05f7fb334e8a0c53e7cea11beb036c Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Wed, 4 May 2022 00:01:40 +0200 Subject: [PATCH 3/3] move env assignment to createServer --- packages/next/server/next.ts | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/next/server/next.ts b/packages/next/server/next.ts index 7e7e787d7f9b2..f2d46252a9a68 100644 --- a/packages/next/server/next.ts +++ b/packages/next/server/next.ts @@ -36,7 +36,6 @@ export class NextServer { private server?: Server private reqHandlerPromise?: Promise private preparedAssetPrefix?: string - private reactRootEnabled?: boolean public options: NextServerOptions constructor(options: NextServerOptions) { @@ -52,17 +51,6 @@ export class NextServer { } getRequestHandler(): RequestHandler { - // Make sure env of custom server is overridden - if (this.reactRootEnabled === undefined) { - // Use dynamic require to make sure it's executed in it's own context - const ReactDOMServer = require('react-dom/server.browser') - this.reactRootEnabled = !!ReactDOMServer.renderToReadableStream - - if (this.reactRootEnabled) { - ;(process.env as any).__NEXT_REACT_ROOT = 'true' - } - } - return async ( req: IncomingMessage, res: ServerResponse, @@ -195,6 +183,14 @@ function createServer(options: NextServerOptions): NextServer { ) } + // Make sure env of custom server is overridden. + // Use dynamic require to make sure it's executed in it's own context. + const ReactDOMServer = require('react-dom/server.browser') + const shouldUseReactRoot = !!ReactDOMServer.renderToReadableStream + if (shouldUseReactRoot) { + ;(process.env as any).__NEXT_REACT_ROOT = 'true' + } + return new NextServer(options) }