Fix shared entries/invalidators module scope (#46533)

This ensures we don't keep `entries` and `invalidator` in module scope
directly and nest it under a key specific to each compilation so
multiple `next` instances in the same process don't override one and
another.

## Bug

- [x] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

Closes: https://github.com/vercel/next.js/pull/46432
Fixes: https://github.com/vercel/next.js/issues/45852
This commit is contained in:
JJ Kasper 2023-02-28 10:17:28 -08:00 committed by GitHub
parent d0ba8003dc
commit d49c700d0d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
21 changed files with 297 additions and 52 deletions

View file

@ -11,7 +11,7 @@ import path from 'path'
import { sources } from 'next/dist/compiled/webpack/webpack'
import {
getInvalidator,
entries,
getEntries,
EntryTypes,
} from '../../../server/dev/on-demand-entry-handler'
import { WEBPACK_LAYERS } from '../../../lib/constants'
@ -146,7 +146,7 @@ export class FlightClientEntryPlugin {
})
}
async createClientEntries(compiler: any, compilation: any) {
async createClientEntries(compiler: webpack.Compiler, compilation: any) {
const addClientEntryAndSSRModulesList: Array<
ReturnType<typeof this.injectClientEntryAndSSRModules>
> = []
@ -426,7 +426,7 @@ export class FlightClientEntryPlugin {
)
// Invalidate in development to trigger recompilation
const invalidator = getInvalidator()
const invalidator = getInvalidator(compiler.outputPath)
// Check if any of the entry injections need an invalidation
if (
invalidator &&
@ -611,7 +611,9 @@ export class FlightClientEntryPlugin {
// Add for the client compilation
// Inject the entry to the client compiler.
if (this.dev) {
const entries = getEntries(compiler.outputPath)
const pageKey = COMPILER_NAMES.client + bundlePath
if (!entries[pageKey]) {
entries[pageKey] = {
type: EntryTypes.CHILD_ENTRY,

View file

@ -32,7 +32,7 @@ import { getPathMatch } from '../../shared/lib/router/utils/path-match'
import { findPageFile } from '../lib/find-page-file'
import {
BUILDING,
entries,
getEntries,
EntryTypes,
getInvalidator,
onDemandEntryHandler,
@ -641,6 +641,8 @@ export default class HotReloader {
for (const config of this.activeConfigs) {
const defaultEntry = config.entry
config.entry = async (...args) => {
const outputPath = this.multiCompiler?.outputPath || ''
const entries: ReturnType<typeof getEntries> = getEntries(outputPath)
// @ts-ignore entry is always a function
const entrypoints = await defaultEntry(...args)
const isClientCompilation = config.name === COMPILER_NAMES.client
@ -1157,7 +1159,8 @@ export default class HotReloader {
}
public invalidate() {
return getInvalidator()?.invalidate()
const outputPath = this.multiCompiler?.outputPath
return outputPath && getInvalidator(outputPath)?.invalidate()
}
public async stop(): Promise<void> {

View file

@ -169,16 +169,35 @@ interface ChildEntry extends EntryType {
parentEntries: Set<string>
}
export const entries: {
/**
* The key composed of the compiler name and the page. For example:
* `edge-server/about`
*/
[entryName: string]: Entry | ChildEntry
} = {}
const entriesMap: Map<
string,
{
/**
* The key composed of the compiler name and the page. For example:
* `edge-server/about`
*/
[entryName: string]: Entry | ChildEntry
}
> = new Map()
let invalidator: Invalidator
export const getInvalidator = () => invalidator
// remove /server from end of output for server compiler
const normalizeOutputPath = (dir: string) => dir.replace(/[/\\]server$/, '')
export const getEntries = (
dir: string
): NonNullable<ReturnType<typeof entriesMap['get']>> => {
dir = normalizeOutputPath(dir)
const entries = entriesMap.get(dir) || {}
entriesMap.set(dir, entries)
return entries
}
const invalidators: Map<string, Invalidator> = new Map()
export const getInvalidator = (dir: string) => {
dir = normalizeOutputPath(dir)
return invalidators.get(dir)
}
const doneCallbacks: EventEmitter | null = new EventEmitter()
const lastClientAccessPages = ['']
@ -238,7 +257,10 @@ class Invalidator {
}
}
function disposeInactiveEntries(maxInactiveAge: number) {
function disposeInactiveEntries(
entries: NonNullable<ReturnType<typeof entriesMap['get']>>,
maxInactiveAge: number
) {
Object.keys(entries).forEach((entryKey) => {
const entryData = entries[entryKey]
const { lastActiveTime, status, dispose } = entryData
@ -426,11 +448,19 @@ export function onDemandEntryHandler({
rootDir: string
appDir?: string
}) {
invalidator = new Invalidator(multiCompiler)
let curInvalidator: Invalidator = getInvalidator(
multiCompiler.outputPath
) as any
let curEntries = getEntries(multiCompiler.outputPath) as any
if (!curInvalidator) {
curInvalidator = new Invalidator(multiCompiler)
invalidators.set(multiCompiler.outputPath, curInvalidator)
}
const startBuilding = (compilation: webpack.Compilation) => {
const compilationName = compilation.name as any as CompilerNameValues
invalidator.startBuilding(compilationName)
curInvalidator.startBuilding(compilationName)
}
for (const compiler of multiCompiler.compilers) {
compiler.hooks.make.tap('NextJsOnDemandEntries', startBuilding)
@ -459,7 +489,7 @@ export function onDemandEntryHandler({
for (const compiler of multiCompiler.compilers) {
compiler.hooks.done.tap('NextJsOnDemandEntries', () =>
getInvalidator().doneBuilding([
getInvalidator(compiler.outputPath)?.doneBuilding([
compiler.name as keyof typeof COMPILER_INDEXES,
])
)
@ -489,7 +519,7 @@ export function onDemandEntryHandler({
]
for (const page of pagePaths) {
const entry = entries[page]
const entry = curEntries[page]
if (!entry) {
continue
}
@ -502,13 +532,13 @@ export function onDemandEntryHandler({
doneCallbacks!.emit(page)
}
invalidator.doneBuilding([...COMPILER_KEYS])
getInvalidator(multiCompiler.outputPath)?.doneBuilding([...COMPILER_KEYS])
})
const pingIntervalTime = Math.max(1000, Math.min(5000, maxInactiveAge))
setInterval(function () {
disposeInactiveEntries(maxInactiveAge)
disposeInactiveEntries(curEntries, maxInactiveAge)
}, pingIntervalTime + 1000).unref()
function handleAppDirPing(
@ -524,7 +554,7 @@ export function onDemandEntryHandler({
COMPILER_NAMES.edgeServer,
]) {
const pageKey = `${compilerType}/${page}`
const entryInfo = entries[pageKey]
const entryInfo = curEntries[pageKey]
// If there's no entry, it may have been invalidated and needs to be re-built.
if (!entryInfo) {
@ -564,7 +594,7 @@ export function onDemandEntryHandler({
COMPILER_NAMES.edgeServer,
]) {
const pageKey = `${compilerType}${page}`
const entryInfo = entries[pageKey]
const entryInfo = curEntries[pageKey]
// If there's no entry, it may have been invalidated and needs to be re-built.
if (!entryInfo) {
@ -644,15 +674,15 @@ export function onDemandEntryHandler({
} => {
const entryKey = `${compilerType}${pagePathData.page}`
if (
entries[entryKey] &&
curEntries[entryKey] &&
// there can be an overlap in the entryKey for the instrumentation hook file and a page named the same
// this is a quick fix to support this scenario by overwriting the instrumentation hook entry, since we only use it one time
// any changes to the instrumentation hook file will require a restart of the dev server anyway
!isInstrumentationHookFilename(entries[entryKey].bundlePath)
!isInstrumentationHookFilename(curEntries[entryKey].bundlePath)
) {
entries[entryKey].dispose = false
entries[entryKey].lastActiveTime = Date.now()
if (entries[entryKey].status === BUILT) {
curEntries[entryKey].dispose = false
curEntries[entryKey].lastActiveTime = Date.now()
if (curEntries[entryKey].status === BUILT) {
return {
entryKey,
newEntry: false,
@ -667,7 +697,7 @@ export function onDemandEntryHandler({
}
}
entries[entryKey] = {
curEntries[entryKey] = {
type: EntryTypes.ENTRY,
appPaths,
absolutePagePath: pagePathData.absolutePagePath,
@ -714,11 +744,11 @@ export function onDemandEntryHandler({
added.set(COMPILER_NAMES.server, addEntry(COMPILER_NAMES.server))
const edgeServerEntry = `${COMPILER_NAMES.edgeServer}${pagePathData.page}`
if (
entries[edgeServerEntry] &&
curEntries[edgeServerEntry] &&
!isInstrumentationHookFile(pagePathData.page)
) {
// Runtime switched from edge to server
delete entries[edgeServerEntry]
delete curEntries[edgeServerEntry]
}
},
onEdgeServer: () => {
@ -728,11 +758,11 @@ export function onDemandEntryHandler({
)
const serverEntry = `${COMPILER_NAMES.server}${pagePathData.page}`
if (
entries[serverEntry] &&
curEntries[serverEntry] &&
!isInstrumentationHookFile(pagePathData.page)
) {
// Runtime switched from server to edge
delete entries[serverEntry]
delete curEntries[serverEntry]
}
},
})
@ -764,7 +794,7 @@ export function onDemandEntryHandler({
})
}
)
invalidator.invalidate([...added.keys()])
curInvalidator.invalidate([...added.keys()])
await Promise.all(invalidatePromises)
}
} finally {

View file

@ -32,7 +32,7 @@ export function getMaybePagePath(
locales?: string[],
appDirEnabled?: boolean
): string | null {
const cacheKey = `${page}:${locales}`
const cacheKey = `${page}:${distDir}:${locales}`
if (pagePathCache.has(cacheKey)) {
return pagePathCache.get(cacheKey) as string | null

View file

@ -0,0 +1,3 @@
module.exports = {
basePath: '/first',
}

View file

@ -0,0 +1,5 @@
{
"version": "0.0.1",
"private": true,
"name": "first-app"
}

View file

@ -0,0 +1,12 @@
export default function Page() {
return <p>hello from first app /blog/[slug]</p>
}
export function getServerSideProps({ params }) {
return {
props: {
now: Date.now(),
params,
},
}
}

View file

@ -0,0 +1,3 @@
export default function Page() {
return <p>hello from first app</p>
}

View file

@ -0,0 +1,19 @@
{
"compilerOptions": {
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": false,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"incremental": true,
"esModuleInterop": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve"
},
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
"exclude": ["node_modules"]
}

View file

@ -0,0 +1,3 @@
module.exports = {
basePath: '/second',
}

View file

@ -0,0 +1,5 @@
{
"version": "0.0.1",
"private": true,
"name": "second-app"
}

View file

@ -0,0 +1,12 @@
export default function Page() {
return <p>hello from second app /another/[slug]</p>
}
export function getServerSideProps({ params }) {
return {
props: {
now: Date.now(),
params,
},
}
}

View file

@ -0,0 +1,12 @@
export default function Page() {
return <p>hello from second app /blog/[slug]</p>
}
export function getServerSideProps({ params }) {
return {
props: {
now: Date.now(),
params,
},
}
}

View file

@ -0,0 +1,3 @@
export default function Page() {
return <p>hello from second app</p>
}

View file

@ -0,0 +1,19 @@
{
"compilerOptions": {
"lib": ["dom", "dom.iterable", "esnext"],
"allowJs": true,
"skipLibCheck": true,
"strict": false,
"forceConsistentCasingInFileNames": true,
"noEmit": true,
"incremental": true,
"esModuleInterop": true,
"module": "esnext",
"moduleResolution": "node",
"resolveJsonModule": true,
"isolatedModules": true,
"jsx": "preserve"
},
"include": ["next-env.d.ts", "**/*.ts", "**/*.tsx"],
"exclude": ["node_modules"]
}

View file

@ -0,0 +1,17 @@
{
"name": "with-zones",
"version": "1.0.0",
"private": true,
"scripts": {
"dev": "node server.js",
"build": "yarn next build apps/first && yarn next build apps/second",
"start": "NODE_ENV=production node server.js"
},
"dependencies": {
"@types/react": "18.0.28",
"next": "canary",
"react": "^18.2.0",
"react-dom": "^18.2.0",
"typescript": "^4.9.3"
}
}

View file

@ -0,0 +1,50 @@
const next = require('next')
const path = require('path')
const { parse } = require('url')
const http = require('http')
;(async () => {
const requestHandlers = new Map()
const dev = process.env.NODE_ENV !== 'production'
for (const appName of ['first', 'second']) {
const appDir = path.join(__dirname, 'apps', appName)
const nextApp = next({
dir: appDir,
dev,
})
await nextApp.prepare()
const handler = nextApp.getRequestHandler()
requestHandlers.set(appName, handler)
}
const server = http.createServer(async (req, res) => {
const appName = req.url?.split('/')[1].split('?')[0]
const handler = requestHandlers.get(appName)
if (!handler) {
res.statusCode = 404
return res.end('not found')
}
try {
await handler(req, res, parse(req.url, true))
} catch (err) {
console.error(err)
res.statusCode = 500
res.end('internal error')
}
})
const parsedPort = Number(process.env.PORT)
const port = !isNaN(parsedPort) ? parsedPort : 3000
server.listen(port, () => {
const actualPort = server.address().port
console.log(
`> started server on url: http://localhost:${actualPort} as ${
dev ? 'development' : process.env.NODE_ENV
}`
)
})
})()

View file

@ -0,0 +1,49 @@
import { createNextDescribe } from 'e2e-utils'
import path from 'path'
createNextDescribe(
'multi-zone',
{
files: path.join(__dirname, 'app'),
skipDeployment: true,
buildCommand: 'pnpm build',
startCommand: (global as any).isNextDev ? 'pnpm dev' : 'pnpm start',
packageJson: {
scripts: {
'post-build': 'echo done',
},
},
},
({ next }) => {
it.each([
{ pathname: '/first', content: ['hello from first app'] },
{ pathname: '/second', content: ['hello from second app'] },
{
pathname: '/first/blog/post-1',
content: ['hello from first app /blog/[slug]'],
},
{
pathname: '/second/blog/post-1',
content: ['hello from second app /blog/[slug]'],
},
{
pathname: '/second/another/post-1',
content: ['hello from second app /another/[slug]'],
},
])(
'should correctly respond for $pathname',
async ({ pathname, content }) => {
const res = await next.fetch(pathname, {
redirect: 'manual',
})
expect(res.status).toBe(200)
const html = await res.text()
for (const item of content) {
expect(html).toContain(item)
}
}
)
}
)

View file

@ -163,13 +163,13 @@ export class NextInstance {
require('next/package.json').version,
},
scripts: {
// since we can't get the build id as a build artifact, make it
// available under the static files
'post-build': 'cp .next/BUILD_ID .next/static/__BUILD_ID',
...pkgScripts,
build:
(pkgScripts['build'] || this.buildCommand || 'next build') +
' && pnpm post-build',
// since we can't get the build id as a build artifact, make it
// available under the static files
'post-build': 'cp .next/BUILD_ID .next/static/__BUILD_ID',
},
},
null,

View file

@ -79,11 +79,7 @@ export class NextDevInstance extends NextInstance {
if (msg.includes('started server on') && msg.includes('url:')) {
// turbo devserver emits stdout in rust directly, can contain unexpected chars with color codes
// strip out again for the safety
this._url = msg
.split('url: ')
.pop()
.trim()
.split(require('os').EOL)[0]
this._url = msg.split('url: ').pop().split(/\s/)[0].trim()
try {
this._parsedUrl = new URL(this._url)
} catch (err) {

View file

@ -87,14 +87,16 @@ export class NextStartInstance extends NextInstance {
})
this._buildId = (
await fs.readFile(
path.join(
this.testDir,
this.nextConfig?.distDir || '.next',
'BUILD_ID'
),
'utf8'
)
await fs
.readFile(
path.join(
this.testDir,
this.nextConfig?.distDir || '.next',
'BUILD_ID'
),
'utf8'
)
.catch(() => '')
).trim()
console.log('running', startArgs.join(' '))
@ -120,7 +122,7 @@ export class NextStartInstance extends NextInstance {
const readyCb = (msg) => {
if (msg.includes('started server on') && msg.includes('url:')) {
this._url = msg.split('url: ').pop().trim()
this._url = msg.split('url: ').pop().split(/\s/)[0].trim()
this._parsedUrl = new URL(this._url)
this.off('stdout', readyCb)
resolve()