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.

Sign up for GitHub

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

Jump to bottom

Fix disposing active entries in dev compilers #39845

Merged
merged 1 commit into from Aug 23, 2022
Merged

Fix disposing active entries in dev compilers #39845

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
96 changes: 57 additions & 39 deletions packages/next/server/dev/on-demand-entry-handler.ts
View file
Open in desktop
Expand Up @@ -453,65 +453,83 @@ export function onDemandEntryHandler({
tree: FlightRouterState
): { success: true } | { invalid: true } {
const pages = getEntrypointsFromTree(tree, true)
let toSend: { invalid: true } | { success: true } = { invalid: true }

for (const page of pages) {
const pageKey = `server/${page}`
for (const compilerType of [
COMPILER_NAMES.client,
COMPILER_NAMES.server,
COMPILER_NAMES.edgeServer,
]) {
const pageKey = `${compilerType}/${page}`
const entryInfo = entries[pageKey]

// If there's no entry, it may have been invalidated and needs to be re-built.
if (!entryInfo) {
// if (page !== lastEntry) client pings, but there's no entry for page
continue
}

// We don't need to maintain active state of anything other than BUILT entries
if (entryInfo.status !== BUILT) continue

// If there's an entryInfo
if (!lastServerAccessPagesForAppDir.includes(pageKey)) {
lastServerAccessPagesForAppDir.unshift(pageKey)

// Maintain the buffer max length
// TODO: verify that the current pageKey is not at the end of the array as multiple entrypoints can exist
if (lastServerAccessPagesForAppDir.length > pagesBufferLength) {
lastServerAccessPagesForAppDir.pop()
}
}
entryInfo.lastActiveTime = Date.now()
entryInfo.dispose = false
toSend = { success: true }
}
}
return toSend
}

function handlePing(pg: string) {
const page = normalizePathSep(pg)
let toSend: { invalid: true } | { success: true } = { invalid: true }

for (const compilerType of [
COMPILER_NAMES.client,
COMPILER_NAMES.server,
COMPILER_NAMES.edgeServer,
]) {
const pageKey = `${compilerType}${page}`
const entryInfo = entries[pageKey]

// If there's no entry, it may have been invalidated and needs to be re-built.
if (!entryInfo) {
// if (page !== lastEntry) client pings, but there's no entry for page
return { invalid: true }
if (compilerType === COMPILER_NAMES.client) {
return { invalid: true }
}
continue
}

// 404 is an on demand entry but when a new page is added we have to refresh the page
toSend = page === '/_error' ? { invalid: true } : { success: true }

// We don't need to maintain active state of anything other than BUILT entries
if (entryInfo.status !== BUILT) continue

// If there's an entryInfo
if (!lastServerAccessPagesForAppDir.includes(pageKey)) {
lastServerAccessPagesForAppDir.unshift(pageKey)
if (!lastClientAccessPages.includes(pageKey)) {
lastClientAccessPages.unshift(pageKey)

// Maintain the buffer max length
// TODO: verify that the current pageKey is not at the end of the array as multiple entrypoints can exist
if (lastServerAccessPagesForAppDir.length > pagesBufferLength) {
lastServerAccessPagesForAppDir.pop()
if (lastClientAccessPages.length > pagesBufferLength) {
lastClientAccessPages.pop()
}
}
entryInfo.lastActiveTime = Date.now()
entryInfo.dispose = false
}

return { success: true }
}

function handlePing(pg: string) {
const page = normalizePathSep(pg)
const pageKey = `client${page}`
const entryInfo = entries[pageKey]

// If there's no entry, it may have been invalidated and needs to be re-built.
if (!entryInfo) {
// if (page !== lastEntry) client pings, but there's no entry for page
return { invalid: true }
}

// 404 is an on demand entry but when a new page is added we have to refresh the page
const toSend = page === '/_error' ? { invalid: true } : { success: true }

// We don't need to maintain active state of anything other than BUILT entries
if (entryInfo.status !== BUILT) return

// If there's an entryInfo
if (!lastClientAccessPages.includes(pageKey)) {
lastClientAccessPages.unshift(pageKey)

// Maintain the buffer max length
if (lastClientAccessPages.length > pagesBufferLength) {
lastClientAccessPages.pop()
}
}
entryInfo.lastActiveTime = Date.now()
entryInfo.dispose = false
return toSend
}

Expand Down
6 changes: 5 additions & 1 deletion test/development/basic/gssp-ssr-change-reloading/test/index.test.ts
View file
Open in desktop
Expand Up @@ -3,7 +3,7 @@
import { join } from 'path'
import webdriver from 'next-webdriver'
import { createNext, FileRef } from 'e2e-utils'
import { check, getRedboxHeader, hasRedbox } from 'next-test-utils'
import { check, getRedboxHeader, hasRedbox, waitFor } from 'next-test-utils'
import { NextInstance } from 'test/lib/next-modes/base'

const installCheckVisible = (browser) => {
Expand Down Expand Up @@ -319,6 +319,10 @@ describe('GS(S)P Server-Side Change Reloading', () => {
expect(props.count).toBe(1)
expect(props.data).toEqual({ hello: 'world' })

// wait longer than the max inactive age for on-demand entries
// to ensure we aren't incorrectly disposing the active entry
await waitFor(20 * 1000)

const page = 'lib/data.json'
const originalContent = await next.readFile(page)

Expand Down