Skip to content

Commit

Permalink
Remove render workers in favor of esm loader (#54813)
Browse files Browse the repository at this point in the history
Currently we create separate workers to isolate `pages` and `app`
routers due to differing react versions being used between the two. This
adds overhead and complexity in the rendering process which we can avoid
by leveraging an `esm-loader` similar to our `require-hook` to properly
alias `pages` router to the bundled react version to match `app` router
when both are leveraged together.

This aims to seamlessly inject the `esm-loader` by restarting the
process with the loader arg added whenever `next` is imported so that
this also works with custom-servers and fixes the issue with custom
req/res fields not working after upgrading.


x-ref: #53883
x-ref: #54288
x-ref: #54129
x-ref: #54435
closes: #54440
closes: #52702
x-ref: [slack
thread](https://vercel.slack.com/archives/C03KAR5DCKC/p1693348443932499?thread_ts=1693275196.347509&cid=C03KAR5DCKC)

---------

Co-authored-by: Tim Neutkens <[email protected]>
Co-authored-by: Zack Tanner <[email protected]>
Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
4 people authored Sep 11, 2023
1 parent e486d74 commit 7d93808
Show file tree
Hide file tree
Showing 59 changed files with 1,073 additions and 972 deletions.
92 changes: 74 additions & 18 deletions packages/next/src/bin/next.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,18 @@
#!/usr/bin/env node
import '../server/require-hook'
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 { commands } from '../lib/commands'
;['react', 'react-dom'].forEach((dependency) => {
try {
// When 'npm link' is used it checks the clone location. Not the project.
require.resolve(dependency)
} catch (err) {
console.warn(
`The module '${dependency}' was not found. Next.js requires that you include it in 'dependencies' of your 'package.json'. To add it, run 'npm install ${dependency}'`
)
}
})
import { commandArgs } from '../lib/command-args'
import loadConfig from '../server/config'
import {
PHASE_PRODUCTION_SERVER,
PHASE_DEVELOPMENT_SERVER,
} from '../shared/lib/constants'
import { getProjectDir } from '../lib/get-project-dir'
import { getValidatedArgs } from '../lib/get-validated-args'
import { findPagesDir } from '../lib/find-pages-dir'

const defaultCommand = 'dev'
const args = arg(
Expand Down Expand Up @@ -122,13 +122,69 @@ if (!process.env.NEXT_MANUAL_SIG_HANDLE && command !== 'dev') {
process.on('SIGTERM', () => process.exit(0))
process.on('SIGINT', () => process.exit(0))
}
async function main() {
const currentArgsSpec = commandArgs[command]()
const validatedArgs = getValidatedArgs(currentArgsSpec, forwardedArgs)

if (
(command === 'start' || command === 'dev') &&
!process.env.NEXT_PRIVATE_WORKER
) {
const dir = getProjectDir(
process.env.NEXT_PRIVATE_DEV_DIR || validatedArgs._[0]
)
process.env.NEXT_PRIVATE_DIR = dir
const origEnv = Object.assign({}, process.env)

// TODO: set config to env variable to be re-used so we don't reload
// un-necessarily
const config = await loadConfig(
command === 'dev' ? PHASE_DEVELOPMENT_SERVER : PHASE_PRODUCTION_SERVER,
dir
)
let dirsResult: ReturnType<typeof findPagesDir> | undefined = undefined

try {
dirsResult = findPagesDir(dir)
} catch (_) {
// handle this error further down
}

commands[command]()
.then((exec) => exec(forwardedArgs))
.then(() => {
if (command === 'build' || command === 'experimental-compile') {
// ensure process exits after build completes so open handles/connections
// don't cause process to hang
process.exit(0)
if (dirsResult?.appDir || process.env.NODE_ENV === 'development') {
process.env = origEnv
}
})

if (dirsResult?.appDir) {
// we need to reset env if we are going to create
// the worker process with the esm loader so that the
// initial env state is correct
process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = config.experimental
.serverActions
? 'experimental'
: 'next'
}
}

for (const dependency of ['react', 'react-dom']) {
try {
// When 'npm link' is used it checks the clone location. Not the project.
require.resolve(dependency)
} catch (err) {
console.warn(
`The module '${dependency}' was not found. Next.js requires that you include it in 'dependencies' of your 'package.json'. To add it, run 'npm install ${dependency}'`
)
}
}

await commands[command]()
.then((exec) => exec(validatedArgs))
.then(() => {
if (command === 'build' || command === 'experimental-compile') {
// ensure process exits after build completes so open handles/connections
// don't cause process to hang
process.exit(0)
}
})
}

main()
70 changes: 44 additions & 26 deletions packages/next/src/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,10 @@ import {
baseOverrides,
defaultOverrides,
experimentalOverrides,
} from '../server/require-hook'
import { initialize } from '../server/lib/incremental-cache-server'
} from '../server/import-overrides'
import { initialize as initializeIncrementalCache } from '../server/lib/incremental-cache-server'
import { nodeFs } from '../server/lib/node-fs-methods'
import { getEsmLoaderPath } from '../server/lib/get-esm-loader-path'

export type SsgRoute = {
initialRevalidateSeconds: number | false
Expand Down Expand Up @@ -1207,9 +1208,8 @@ export default async function build(
: config.experimental.cpus || 4

function createStaticWorker(
type: 'app' | 'pages',
ipcPort: number,
ipcValidationKey: string
incrementalCacheIpcPort: number,
incrementalCacheIpcValidationKey: string
) {
let infoPrinted = false

Expand Down Expand Up @@ -1246,16 +1246,21 @@ export default async function build(
},
numWorkers,
forkOptions: {
execArgv: [
'--experimental-loader',
getEsmLoaderPath(),
'--no-warnings',
],
env: {
...process.env,
__NEXT_INCREMENTAL_CACHE_IPC_PORT: ipcPort + '',
__NEXT_INCREMENTAL_CACHE_IPC_KEY: ipcValidationKey,
__NEXT_PRIVATE_PREBUNDLED_REACT:
type === 'app'
? config.experimental.serverActions
? 'experimental'
: 'next'
: undefined,
__NEXT_INCREMENTAL_CACHE_IPC_PORT: incrementalCacheIpcPort + '',
__NEXT_INCREMENTAL_CACHE_IPC_KEY:
incrementalCacheIpcValidationKey,
__NEXT_PRIVATE_PREBUNDLED_REACT: hasAppDir
? config.experimental.serverActions
? 'experimental'
: 'next'
: '',
},
},
enableWorkerThreads: config.experimental.workerThreads,
Expand Down Expand Up @@ -1290,7 +1295,10 @@ export default async function build(
CacheHandler = CacheHandler.default || CacheHandler
}

const { ipcPort, ipcValidationKey } = await initialize({
const {
ipcPort: incrementalCacheIpcPort,
ipcValidationKey: incrementalCacheIpcValidationKey,
} = await initializeIncrementalCache({
fs: nodeFs,
dev: false,
appDir: isAppDirEnabled,
Expand All @@ -1315,12 +1323,14 @@ export default async function build(
})

const pagesStaticWorkers = createStaticWorker(
'pages',
ipcPort,
ipcValidationKey
incrementalCacheIpcPort,
incrementalCacheIpcValidationKey
)
const appStaticWorkers = isAppDirEnabled
? createStaticWorker('app', ipcPort, ipcValidationKey)
? createStaticWorker(
incrementalCacheIpcPort,
incrementalCacheIpcValidationKey
)
: undefined

const analysisBegin = process.hrtime()
Expand Down Expand Up @@ -2124,9 +2134,14 @@ export default async function build(

const vanillaServerEntries = [
...sharedEntriesSet,
isStandalone
? require.resolve('next/dist/server/lib/start-server')
: null,
...(isStandalone
? [
require.resolve('next/dist/server/lib/start-server'),
require.resolve('next/dist/server/next'),
require.resolve('next/dist/esm/server/esm-loader.mjs'),
require.resolve('next/dist/server/import-overrides'),
]
: []),
require.resolve('next/dist/server/next-server'),
].filter(Boolean) as string[]

Expand Down Expand Up @@ -2402,7 +2417,8 @@ export default async function build(
outputFileTracingRoot,
requiredServerFiles.config,
middlewareManifest,
hasInstrumentationHook
hasInstrumentationHook,
hasAppDir
)
})
}
Expand Down Expand Up @@ -3315,11 +3331,13 @@ export default async function build(
require('../export').default

const pagesWorker = createStaticWorker(
'pages',
ipcPort,
ipcValidationKey
incrementalCacheIpcPort,
incrementalCacheIpcValidationKey
)
const appWorker = createStaticWorker(
incrementalCacheIpcPort,
incrementalCacheIpcValidationKey
)
const appWorker = createStaticWorker('app', ipcPort, ipcValidationKey)

const options: ExportOptions = {
isInvokedFromCli: false,
Expand Down
21 changes: 13 additions & 8 deletions packages/next/src/build/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1830,7 +1830,8 @@ export async function copyTracedFiles(
tracingRoot: string,
serverConfig: { [key: string]: any },
middlewareManifest: MiddlewareManifest,
hasInstrumentationHook: boolean
hasInstrumentationHook: boolean,
hasAppDir: boolean
) {
const outputPath = path.join(distDir, 'standalone')
let moduleType = false
Expand Down Expand Up @@ -1963,12 +1964,11 @@ export async function copyTracedFiles(
moduleType
? `import path from 'path'
import { fileURLToPath } from 'url'
import module from 'module'
const require = module.createRequire(import.meta.url)
const __dirname = fileURLToPath(new URL('.', import.meta.url))
import { startServer } from 'next/dist/server/lib/start-server.js'
`
: `
const path = require('path')
const { startServer } = require('next/dist/server/lib/start-server')`
: `const path = require('path')`
}
const dir = path.join(__dirname)
Expand All @@ -1993,9 +1993,14 @@ const nextConfig = ${JSON.stringify({
})}
process.env.__NEXT_PRIVATE_STANDALONE_CONFIG = JSON.stringify(nextConfig)
process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = nextConfig.experimental && nextConfig.experimental.serverActions
? 'experimental'
: 'next'
process.env.__NEXT_PRIVATE_PREBUNDLED_REACT = ${hasAppDir}
? nextConfig.experimental && nextConfig.experimental.serverActions
? 'experimental'
: 'next'
: '';
require('next')
const { startServer } = require('next/dist/server/lib/start-server')
if (
Number.isNaN(keepAliveTimeout) ||
Expand Down
31 changes: 22 additions & 9 deletions packages/next/src/build/webpack-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import { NextFontManifestPlugin } from './webpack/plugins/next-font-manifest-plu
import { getSupportedBrowsers } from './utils'
import { MemoryWithGcCachePlugin } from './webpack/plugins/memory-with-gc-cache-plugin'
import { getBabelConfigFile } from './get-babel-config-file'
import { defaultOverrides } from '../server/import-overrides'

type ExcludesFalse = <T>(x: T | false) => x is T
type ClientEntries = {
Expand Down Expand Up @@ -1127,6 +1128,14 @@ export default async function getBaseWebpackConfig(
'@opentelemetry/api': 'next/dist/compiled/@opentelemetry/api',
}),

...(hasAppDir
? createRSCAliases(bundledReactChannel, {
reactSharedSubset: false,
reactDomServerRenderingStub: false,
reactProductionProfiling,
})
: {}),

...(config.images.loaderFile
? {
'next/dist/shared/lib/image-loader': config.images.loaderFile,
Expand All @@ -1138,8 +1147,8 @@ export default async function getBaseWebpackConfig(

next: NEXT_PROJECT_ROOT,

'styled-jsx/style$': require.resolve(`styled-jsx/style`),
'styled-jsx$': require.resolve(`styled-jsx`),
'styled-jsx/style$': defaultOverrides['styled-jsx/style'],
'styled-jsx$': defaultOverrides['styled-jsx'],

...customAppAliases,
...customErrorAlias,
Expand Down Expand Up @@ -1273,7 +1282,16 @@ export default async function getBaseWebpackConfig(
}
}

for (const packageName of ['react', 'react-dom']) {
for (const packageName of [
'react',
'react-dom',
...(hasAppDir
? [
`next/dist/compiled/react${bundledReactChannel}`,
`next/dist/compiled/react-dom${bundledReactChannel}`,
]
: []),
]) {
addPackagePath(packageName, dir)
}

Expand Down Expand Up @@ -1541,7 +1559,7 @@ export default async function getBaseWebpackConfig(
// Forcedly resolve the styled-jsx installed by next.js,
// since `resolveExternal` cannot find the styled-jsx dep with pnpm
if (request === 'styled-jsx/style') {
resolveResult.res = require.resolve(request)
resolveResult.res = defaultOverrides['styled-jsx/style']
}

const { res, isEsm } = resolveResult
Expand Down Expand Up @@ -2116,11 +2134,6 @@ export default async function getBaseWebpackConfig(
[require.resolve('next/dynamic')]: require.resolve(
'next/dist/shared/lib/app-dynamic'
),
...createRSCAliases(bundledReactChannel, {
reactSharedSubset: false,
reactDomServerRenderingStub: false,
reactProductionProfiling,
}),
},
},
},
Expand Down
Loading

0 comments on commit 7d93808

Please sign in to comment.