fix: react dev bundle is picked in prod mode (#39221)

When we detect if `reactRoot` rendering should be enabled we `require` the require to check the version. But at that time the `NODE_ENV` isn't set yet. Then the react dev build stays in the `require.cache` that any future require of react will get the wrong build. In that case, React dev bundle is picked in production mode.

Fun fact: if you're using hooks, that seem not to effect you, but context consumer works different then you couldn't get the proper context from provider.

Fixes #38176
Fixes #38765
Fixes #38332

## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
This commit is contained in:
Jiachi Liu 2022-08-01 16:21:42 +02:00 committed by GitHub
parent 582cb3766d
commit e9d23d709c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 66 additions and 66 deletions

View file

@ -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 '../server/utils'
;['react', 'react-dom'].forEach((dependency) => {
try {
// When 'npm link' is used it checks the clone location. Not the project.
@ -107,6 +106,9 @@ if (process.env.NODE_ENV) {
;(process.env as any).NODE_ENV = process.env.NODE_ENV || defaultEnv
;(process.env as any).NEXT_RUNTIME = 'nodejs'
// In node.js runtime, react has to be required after NODE_ENV is set,
// so that the correct dev/prod bundle could be loaded into require.cache.
const { shouldUseReactRoot } = require('../server/utils')
if (shouldUseReactRoot) {
;(process.env as any).__NEXT_REACT_ROOT = 'true'
}

View file

@ -0,0 +1,17 @@
import React from 'react'
const NumberContext = React.createContext(0)
export default function page() {
return (
<NumberContext.Provider value={12345}>
<NumberContext.Consumer>
{(value) => <p>Value: {value}</p>}
</NumberContext.Consumer>
</NumberContext.Provider>
)
}
export async function getServerSideProps() {
return { props: {} }
}

View file

@ -0,0 +1,46 @@
import { join } from 'path'
import { renderViaHTTP, check } from 'next-test-utils'
import { NextInstance } from 'test/lib/next-modes/base'
import { createNext, FileRef } from 'e2e-utils'
describe('React Context', () => {
let next: NextInstance
beforeAll(async () => {
next = await createNext({
files: {
pages: new FileRef(join(__dirname, 'app/pages')),
'context.js': new FileRef(join(__dirname, 'app/context.js')),
},
})
})
afterAll(() => next.destroy())
it('should render a page with context', async () => {
const html = await renderViaHTTP(next.url, '/')
expect(html).toMatch(/Value: .*?hello world/)
})
it('should render correctly with context consumer', async () => {
const html = await renderViaHTTP(next.url, '/consumer')
expect(html).toMatch(/Value: .*?12345/)
})
if ((globalThis as any).isNextDev) {
it('should render with context after change', async () => {
const aboutAppPagePath = 'pages/_app.js'
const originalContent = await next.readFile(aboutAppPagePath)
await next.patchFile(
aboutAppPagePath,
originalContent.replace('hello world', 'new value')
)
try {
await check(() => renderViaHTTP(next.url, '/'), /Value: .*?new value/)
} finally {
await next.patchFile(aboutAppPagePath, originalContent)
}
await check(() => renderViaHTTP(next.url, '/'), /Value: .*?hello world/)
})
}
})

View file

@ -1,65 +0,0 @@
/* eslint-env jest */
import { join } from 'path'
import {
File,
killApp,
findPort,
nextStart,
nextBuild,
renderViaHTTP,
check,
launchApp,
} from 'next-test-utils'
const appDir = join(__dirname, '../')
const appPg = new File(join(appDir, 'pages/_app.js'))
let appPort
let app
const runTests = (isDev) => {
it('should render a page with context', async () => {
const html = await renderViaHTTP(appPort, '/')
expect(html).toMatch(/Value: .*?hello world/)
})
if (isDev) {
it('should render with context after change', async () => {
appPg.replace('hello world', 'new value')
try {
await check(() => renderViaHTTP(appPort, '/'), /Value: .*?new value/)
} finally {
appPg.restore()
}
await check(() => renderViaHTTP(appPort, '/'), /Value: .*?hello world/)
})
}
}
describe('React Context', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(async () => {
await killApp(app)
appPg.restore()
})
runTests(true)
})
describe('production mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))
runTests()
})
})