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

Update to handle correct react version with app #41658

Merged
merged 6 commits into from Oct 22, 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
4 changes: 4 additions & 0 deletions packages/next/build/index.ts
Expand Up @@ -120,6 +120,7 @@ import { RemotePattern } from '../shared/lib/image-config'
import { eventSwcPlugins } from '../telemetry/events/swc-plugins'
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
import { AppBuildManifest } from './webpack/plugins/app-build-manifest-plugin'
import { verifyAppReactVersion } from '../lib/verifyAppReactVersion'

export type SsgRoute = {
initialRevalidateSeconds: number | false
Expand Down Expand Up @@ -307,6 +308,9 @@ export default async function build(
}
const { pagesDir, appDir } = findPagesDir(dir, isAppDirEnabled)

if (isAppDirEnabled) {
await verifyAppReactVersion({ dir })
}
const hasPublicDir = await fileExists(publicDir)

telemetry.record(
Expand Down
122 changes: 122 additions & 0 deletions packages/next/lib/verifyAppReactVersion.ts
@@ -0,0 +1,122 @@
import chalk from 'next/dist/compiled/chalk'

import {
hasNecessaryDependencies,
MissingDependency,
NecessaryDependencies,
} from './has-necessary-dependencies'

import { installDependencies } from './install-dependencies'
import { isCI } from '../telemetry/ci-info'
import { FatalError } from './fatal-error'
import { getPkgManager } from './helpers/get-pkg-manager'
import { getOxfordCommaList } from './oxford-comma-list'
const requiredReactVersion = process.env.REQUIRED_APP_REACT_VERSION || ''

const removalMsg =
'\n\n' +
chalk.bold(
'If you are not trying to use the `app` directory, please disable the ' +
chalk.cyan('experimental.appDir') +
' config in your `next.config.js`.'
)

const requiredPackages = [
{
file: 'react/index.js',
pkg: 'react',
exportsRestrict: true,
},
{
file: 'react-dom/index.js',
pkg: 'react-dom',
exportsRestrict: true,
},
]

async function missingDepsError(
dir: string,
missingPackages: MissingDependency[]
) {
const packagesHuman = getOxfordCommaList(missingPackages.map((p) => p.pkg))
const packagesCli = missingPackages
.map((p) => `${p.pkg}@${requiredReactVersion}`)
.join(' ')
const packageManager = getPkgManager(dir)

throw new FatalError(
chalk.bold.red(
`It looks like you're trying to use the \`app\` directory but do not have the required react version installed.`
) +
'\n\n' +
chalk.bold(`Please install ${chalk.bold(packagesHuman)} by running:`) +
'\n\n' +
`\t${chalk.bold.cyan(
(packageManager === 'yarn'
? 'yarn add --dev'
: packageManager === 'pnpm'
? 'pnpm install --save-dev'
: 'npm install --save-dev') +
' ' +
packagesCli
)}` +
removalMsg +
'\n'
)
}

export async function verifyAppReactVersion({
dir,
}: {
dir: string
}): Promise<void> {
if (process.env.NEXT_SKIP_APP_REACT_INSTALL) {
return
}

// Ensure TypeScript and necessary `@types/*` are installed:
let deps: NecessaryDependencies = await hasNecessaryDependencies(
dir,
requiredPackages
)
const resolvedReact = deps.resolved.get('react')
const installedVersion =
resolvedReact &&
require(deps.resolved.get('react') || '')
.version?.split('-experimental')
.pop()

if (
deps.missing?.length ||
installedVersion !== requiredReactVersion.split('-experimental').pop()
) {
const neededDeps = requiredPackages.map((dep) => {
dep.pkg = `${dep.pkg}@${requiredReactVersion}`
return dep
})

if (isCI) {
// we don't attempt auto install in CI to avoid side-effects
// and instead log the error for installing needed packages
await missingDepsError(dir, neededDeps)
}
console.log(
chalk.bold.yellow(
`It looks like you're trying to use \`app\` directory but do not have the required react version installed.`
) +
'\n' +
removalMsg +
'\n'
)
await installDependencies(dir, neededDeps, true).catch((err) => {
if (err && typeof err === 'object' && 'command' in err) {
console.error(
`\nFailed to install required react versions, please install them manually to continue:\n` +
(err as any).command +
'\n'
)
}
process.exit(1)
})
}
}
5 changes: 5 additions & 0 deletions packages/next/server/dev/next-dev-server.ts
Expand Up @@ -76,6 +76,7 @@ import {
} from '../../build/utils'
import { getDefineEnv } from '../../build/webpack-config'
import loadJsConfig from '../../build/load-jsconfig'
import { verifyAppReactVersion } from '../../lib/verifyAppReactVersion'

// Load ReactDevOverlay only when needed
let ReactDevOverlayImpl: FunctionComponent
Expand Down Expand Up @@ -658,6 +659,10 @@ export default class DevServer extends Server {
setGlobal('distDir', this.distDir)
setGlobal('phase', PHASE_DEVELOPMENT_SERVER)

if (this.hasAppDir) {
await verifyAppReactVersion({ dir: this.dir })
}

await this.verifyTypeScript()
this.customRoutes = await loadCustomRoutes(this.nextConfig)

Expand Down
17 changes: 13 additions & 4 deletions packages/next/taskfile-swc.js
Expand Up @@ -156,8 +156,17 @@ if ((typeof exports.default === 'function' || (typeof exports.default === 'objec
}

function setNextVersion(code) {
return code.replace(
/process\.env\.__NEXT_VERSION/g,
`"${require('./package.json').version}"`
)
return code
.replace(
/process\.env\.__NEXT_VERSION/g,
`"${require('./package.json').version}"`
)
.replace(
/process\.env\.REQUIRED_APP_REACT_VERSION/,
`"${
require('../../package.json').devDependencies[
'react-server-dom-webpack'
]
}"`
)
}
4 changes: 2 additions & 2 deletions test/.stats-app/package.json
Expand Up @@ -4,7 +4,7 @@
"license": "MIT",
"dependencies": {
"next": "latest",
"react": "0.0.0-experimental-cb5084d1c-20220924",
"react-dom": "0.0.0-experimental-cb5084d1c-20220924"
"react": "0.0.0-experimental-9cdf8a99e-20221018",
"react-dom": "0.0.0-experimental-9cdf8a99e-20221018"
}
}
Expand Up @@ -8,7 +8,11 @@ const appDir = path.join(__dirname, '..')
describe('conflict between app file and page file', () => {
it('errors during build', async () => {
const conflicts = ['/hello', '/another']
const results = await nextBuild(appDir, [], { stdout: true, stderr: true })
const results = await nextBuild(appDir, [], {
stdout: true,
stderr: true,
env: { NEXT_SKIP_APP_REACT_INSTALL: '1' },
})
const output = results.stdout + results.stderr
expect(output).toMatch(/Conflicting app and page files were found/)

Expand Down