Skip to content

Commit

Permalink
Fix shared entries/invalidators module scope (#46533)
Browse files Browse the repository at this point in the history
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: #46432
Fixes: #45852
  • Loading branch information
ijjk committed Feb 28, 2023
1 parent d0ba800 commit d49c700
Show file tree
Hide file tree
Showing 21 changed files with 297 additions and 52 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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>
> = []
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 5 additions & 2 deletions packages/next/src/server/dev/hot-reloader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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> {
Expand Down
88 changes: 59 additions & 29 deletions packages/next/src/server/dev/on-demand-entry-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()

// 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
}

let invalidator: Invalidator
export const getInvalidator = () => invalidator
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 = ['']
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
])
)
Expand Down Expand Up @@ -489,7 +519,7 @@ export function onDemandEntryHandler({
]

for (const page of pagePaths) {
const entry = entries[page]
const entry = curEntries[page]
if (!entry) {
continue
}
Expand All @@ -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(
Expand All @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -667,7 +697,7 @@ export function onDemandEntryHandler({
}
}

entries[entryKey] = {
curEntries[entryKey] = {
type: EntryTypes.ENTRY,
appPaths,
absolutePagePath: pagePathData.absolutePagePath,
Expand Down Expand Up @@ -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: () => {
Expand All @@ -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]
}
},
})
Expand Down Expand Up @@ -764,7 +794,7 @@ export function onDemandEntryHandler({
})
}
)
invalidator.invalidate([...added.keys()])
curInvalidator.invalidate([...added.keys()])
await Promise.all(invalidatePromises)
}
} finally {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/src/server/require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/multi-zone/app/apps/first/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
basePath: '/first',
}
5 changes: 5 additions & 0 deletions test/e2e/multi-zone/app/apps/first/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"version": "0.0.1",
"private": true,
"name": "first-app"
}
12 changes: 12 additions & 0 deletions test/e2e/multi-zone/app/apps/first/pages/blog/[slug].tsx
Original file line number Diff line number Diff line change
@@ -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,
},
}
}
3 changes: 3 additions & 0 deletions test/e2e/multi-zone/app/apps/first/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>hello from first app</p>
}
19 changes: 19 additions & 0 deletions test/e2e/multi-zone/app/apps/first/tsconfig.json
Original file line number Diff line number Diff line change
@@ -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"]
}
3 changes: 3 additions & 0 deletions test/e2e/multi-zone/app/apps/second/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
basePath: '/second',
}
5 changes: 5 additions & 0 deletions test/e2e/multi-zone/app/apps/second/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"version": "0.0.1",
"private": true,
"name": "second-app"
}
12 changes: 12 additions & 0 deletions test/e2e/multi-zone/app/apps/second/pages/another/[slug].tsx
Original file line number Diff line number Diff line change
@@ -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,
},
}
}
12 changes: 12 additions & 0 deletions test/e2e/multi-zone/app/apps/second/pages/blog/[slug].tsx
Original file line number Diff line number Diff line change
@@ -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,
},
}
}
3 changes: 3 additions & 0 deletions test/e2e/multi-zone/app/apps/second/pages/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export default function Page() {
return <p>hello from second app</p>
}
19 changes: 19 additions & 0 deletions test/e2e/multi-zone/app/apps/second/tsconfig.json
Original file line number Diff line number Diff line change
@@ -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"]
}
17 changes: 17 additions & 0 deletions test/e2e/multi-zone/app/package.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
Loading

0 comments on commit d49c700

Please sign in to comment.