Skip to content
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

Align reactRoot config between server and webpack config #34328

Merged
merged 3 commits into from Feb 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 3 additions & 22 deletions packages/next/build/webpack-config.ts
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
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
@@ -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
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
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
@@ -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
@@ -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
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