Skip to content

Commit

Permalink
Align reactRoot config between server and webpack config (vercel#34328)
Browse files Browse the repository at this point in the history
### Changes

* node server and webpack should share the same logic: auto detect react 18 and enable `reactRoot`
* fallback `_error` should use functional document if concurrent rendering is enabled

### Test

* Remove the hard code `reactRoot: true` in test suite
* Test some react-18 test suite with nodejs runtime
  • Loading branch information
huozhi authored Feb 14, 2022
1 parent 7be6359 commit 1aee935
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 83 deletions.
25 changes: 3 additions & 22 deletions packages/next/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import ReactRefreshWebpackPlugin from 'next/dist/compiled/@next/react-refresh-ut
import chalk from 'next/dist/compiled/chalk'
import crypto from 'crypto'
import { stringify } from 'querystring'
import semver from 'next/dist/compiled/semver'
import { webpack } from 'next/dist/compiled/webpack/webpack'
import type { webpack5 } from 'next/dist/compiled/webpack/webpack'
import path, { join as pathJoin, relative as relativePath } from 'path'
Expand All @@ -15,7 +14,6 @@ import {
MIDDLEWARE_ROUTE,
} from '../lib/constants'
import { fileExists } from '../lib/file-exists'
import { getPackageVersion } from '../lib/get-package-version'
import { CustomRoutes } from '../lib/load-custom-routes.js'
import {
CLIENT_STATIC_FILES_RUNTIME_AMP,
Expand Down Expand Up @@ -52,6 +50,7 @@ import type { Span } from '../trace'
import { getRawPageExtensions } from './utils'
import browserslist from 'next/dist/compiled/browserslist'
import loadJsConfig from './load-jsconfig'
import { shouldUseReactRoot } from '../server/config'

const watchOptions = Object.freeze({
aggregateTimeout: 5,
Expand Down Expand Up @@ -337,21 +336,7 @@ export default async function getBaseWebpackConfig(
rewrites.afterFiles.length > 0 ||
rewrites.fallback.length > 0
const hasReactRefresh: boolean = dev && !isServer
const reactDomVersion = await getPackageVersion({
cwd: dir,
name: 'react-dom',
})
const isReactExperimental = Boolean(
reactDomVersion && /0\.0\.0-experimental/.test(reactDomVersion)
)
const hasReact18: boolean =
Boolean(reactDomVersion) &&
(semver.gte(reactDomVersion!, '18.0.0') ||
semver.coerce(reactDomVersion)?.version === '18.0.0')

const hasReactRoot: boolean =
config.experimental.reactRoot || hasReact18 || isReactExperimental

const hasReactRoot = shouldUseReactRoot()
const runtime = config.experimental.runtime

// Make sure reactRoot is enabled when react 18 is detected
Expand All @@ -360,11 +345,7 @@ export default async function getBaseWebpackConfig(
}

// Only inform during one of the builds
if (
!isServer &&
config.experimental.reactRoot &&
!(hasReact18 || isReactExperimental)
) {
if (!isServer && config.experimental.reactRoot && !hasReactRoot) {
// It's fine to only mention React 18 here as we don't recommend people to try experimental.
Log.warn('You have to use React 18 to use `experimental.reactRoot`.')
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export function getRender({
webServerConfig: {
extendRenderOpts: {
buildId,
reactRoot: true,
runtime: 'edge',
supportsDynamicHTML: true,
disableOptimizedLoading: true,
Expand Down
23 changes: 21 additions & 2 deletions packages/next/server/config.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import chalk from '../lib/chalk'
import findUp from 'next/dist/compiled/find-up'
import { basename, extname, relative, isAbsolute, resolve } from 'path'
import { pathToFileURL } from 'url'
import { Agent as HttpAgent } from 'http'
import { Agent as HttpsAgent } from 'https'
import semver from 'next/dist/compiled/semver'
import findUp from 'next/dist/compiled/find-up'
import chalk from '../lib/chalk'
import * as Log from '../build/output/log'
import { CONFIG_FILES, PHASE_DEVELOPMENT_SERVER } from '../shared/lib/constants'
import { execOnce } from '../shared/lib/utils'
Expand Down Expand Up @@ -653,6 +654,11 @@ export default async function loadConfig(
)
}

const hasReactRoot = shouldUseReactRoot()
if (hasReactRoot) {
userConfig.experimental.reactRoot = true
}

if (userConfig.amp?.canonicalBase) {
const { canonicalBase } = userConfig.amp || ({} as any)
userConfig.amp = userConfig.amp || {}
Expand Down Expand Up @@ -698,6 +704,19 @@ export default async function loadConfig(
return completeConfig
}

export function shouldUseReactRoot() {
const reactDomVersion = require('react-dom').version
const isReactExperimental = Boolean(
reactDomVersion && /0\.0\.0-experimental/.test(reactDomVersion)
)
const hasReact18: boolean =
Boolean(reactDomVersion) &&
(semver.gte(reactDomVersion!, '18.0.0') ||
semver.coerce(reactDomVersion)?.version === '18.0.0')

return hasReact18 || isReactExperimental
}

export function setHttpAgentOptions(
options: NextConfigComplete['httpAgentOptions']
) {
Expand Down
4 changes: 3 additions & 1 deletion packages/next/server/dev/next-dev-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -940,7 +940,9 @@ export default class DevServer extends Server {
// Build the error page to ensure the fallback is built too.
// TODO: See if this can be moved into hotReloader or removed.
await this.hotReloader!.ensurePage('/_error')
return await loadDefaultErrorComponents(this.distDir)
return await loadDefaultErrorComponents(this.distDir, {
hasConcurrentFeatures: !!this.renderOpts.runtime,
})
}

protected setImmutableAssetCacheControl(res: BaseNextResponse): void {
Expand Down
10 changes: 8 additions & 2 deletions packages/next/server/load-components.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,14 @@ export type LoadComponentsReturnType = {
ComponentMod: any
}

export async function loadDefaultErrorComponents(distDir: string) {
const Document = interopDefault(require('next/dist/pages/_document'))
export async function loadDefaultErrorComponents(
distDir: string,
{ hasConcurrentFeatures }: { hasConcurrentFeatures: boolean }
) {
const Document = interopDefault(
require(`next/dist/pages/_document` +
(hasConcurrentFeatures ? '-concurrent' : ''))
)
const App = interopDefault(require('next/dist/pages/_app'))
const ComponentMod = require('next/dist/pages/_error')
const Component = interopDefault(ComponentMod)
Expand Down
107 changes: 56 additions & 51 deletions test/integration/react-18/test/index.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
/* eslint-env jest */

import { join } from 'path'
import fs from 'fs-extra'

import {
File,
Expand Down Expand Up @@ -57,12 +56,11 @@ async function getDevOutput(dir) {

describe('React 18 Support', () => {
describe('Use legacy render', () => {
beforeAll(async () => {
await fs.remove(join(appDir, 'node_modules'))
beforeAll(() => {
nextConfig.replace('reactRoot: true', 'reactRoot: false')
})
afterAll(() => {
nextConfig.replace('reactRoot: false', 'reactRoot: true')
nextConfig.restore()
})

test('supported version of react in dev', async () => {
Expand All @@ -75,15 +73,17 @@ describe('React 18 Support', () => {
expect(output).not.toMatch(USING_CREATE_ROOT)
})

test('suspense is not allowed in blocking rendering mode (prod)', async () => {
test('suspense is not allowed in blocking rendering mode', async () => {
nextConfig.replace('withReact18({', '/*withReact18*/({')
const { stderr, code } = await nextBuild(appDir, [], {
nodeArgs,
stderr: true,
})
expect(code).toBe(1)
nextConfig.replace('/*withReact18*/({', 'withReact18({')

expect(stderr).toContain(
'Invalid suspense option usage in next/dynamic. Read more: https://nextjs.org/docs/messages/invalid-dynamic-suspense'
)
expect(code).toBe(1)
})
})
})
Expand Down Expand Up @@ -119,56 +119,58 @@ describe('Blocking mode', () => {
)
})

describe('Concurrent mode in the edge runtime', () => {
beforeAll(async () => {
nextConfig.replace("// runtime: 'edge'", "runtime: 'edge'")
dynamicHello.replace('suspense = false', `suspense = true`)
// `noSSR` mode will be ignored by suspense
dynamicHello.replace('let ssr', `let ssr = false`)
})
afterAll(async () => {
nextConfig.restore()
dynamicHello.restore()
})
function runTestsAgainstRuntime(runtime) {
describe(`Concurrent mode in the ${runtime} runtime`, () => {
beforeAll(async () => {
nextConfig.replace("// runtime: 'edge'", `runtime: '${runtime}'`)
dynamicHello.replace('suspense = false', `suspense = true`)
// `noSSR` mode will be ignored by suspense
dynamicHello.replace('let ssr', `let ssr = false`)
})
afterAll(async () => {
nextConfig.restore()
dynamicHello.restore()
})

runTests('`runtime` is set to `edge`', (context) => {
concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q))
runTests(`runtime is set to '${runtime}'`, (context) => {
concurrent(context, (p, q) => renderViaHTTP(context.appPort, p, q))

it('should stream to users', async () => {
const res = await fetchViaHTTP(context.appPort, '/ssr')
expect(res.headers.get('etag')).toBeNull()
})
it('should stream to users', async () => {
const res = await fetchViaHTTP(context.appPort, '/ssr')
expect(res.headers.get('etag')).toBeNull()
})

it('should not stream to bots', async () => {
const res = await fetchViaHTTP(
context.appPort,
'/ssr',
{},
{
headers: {
'user-agent': 'Googlebot',
},
}
)
expect(res.headers.get('etag')).toBeDefined()
})
it('should not stream to bots', async () => {
const res = await fetchViaHTTP(
context.appPort,
'/ssr',
{},
{
headers: {
'user-agent': 'Googlebot',
},
}
)
expect(res.headers.get('etag')).toBeDefined()
})

it('should not stream to google pagerender bot', async () => {
const res = await fetchViaHTTP(
context.appPort,
'/ssr',
{},
{
headers: {
'user-agent':
'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Google-PageRenderer Google (+https://developers.google.com/+/web/snippet/)',
},
}
)
expect(res.headers.get('etag')).toBeDefined()
it('should not stream to google pagerender bot', async () => {
const res = await fetchViaHTTP(
context.appPort,
'/ssr',
{},
{
headers: {
'user-agent':
'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/56.0.2924.87 Safari/537.36 Google-PageRenderer Google (+https://developers.google.com/+/web/snippet/)',
},
}
)
expect(res.headers.get('etag')).toBeDefined()
})
})
})
})
}

function runTest(mode, name, fn) {
const context = { appDir }
Expand Down Expand Up @@ -200,6 +202,9 @@ function runTest(mode, name, fn) {
})
}

runTestsAgainstRuntime('edge')
runTestsAgainstRuntime('nodejs')

function runTests(name, fn) {
runTest('dev', name, fn)
runTest('prod', name, fn)
Expand Down
4 changes: 0 additions & 4 deletions test/integration/react-18/test/with-react-18.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,4 @@
module.exports = function withReact18(config) {
if (typeof config.experimental.reactRoot === 'undefined') {
config.experimental.reactRoot = true
}

config.webpack = (webpackConfig) => {
const { alias } = webpackConfig.resolve
// FIXME: resolving react/jsx-runtime https://github.com/facebook/react/issues/20235
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ module.exports = withReact18({
},
pageExtensions: ['js', 'ts', 'jsx'], // .tsx won't be treat as page,
experimental: {
reactRoot: true,
serverComponents: true,
runtime: 'edge',
},
Expand Down

0 comments on commit 1aee935

Please sign in to comment.