From 611e13f5159457fedf96d850845650616a1f75dd Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 23 Aug 2022 02:23:43 -0500 Subject: [PATCH] Fix disposing active entries in dev compilers (#39845) As noticed in https://github.com/markdoc/markdoc/issues/131 it seems we are incorrectly disposing active entries causing HMR to fail after the configured `maxInactiveAge`. To fix this instead of only updating lastActiveTime for one compiler type's entry we update all active compiler types for the active entry. This also updates an existing test to catch this by waiting the `maxInactiveAge` before attempting a change that should trigger HMR. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [x] Errors have helpful link attached, see `contributing.md` Fixes: https://github.com/markdoc/markdoc/issues/131 --- .../server/dev/on-demand-entry-handler.ts | 96 +++++++++++-------- .../test/index.test.ts | 6 +- 2 files changed, 62 insertions(+), 40 deletions(-) diff --git a/packages/next/server/dev/on-demand-entry-handler.ts b/packages/next/server/dev/on-demand-entry-handler.ts index 00561904706e..09570760812f 100644 --- a/packages/next/server/dev/on-demand-entry-handler.ts +++ b/packages/next/server/dev/on-demand-entry-handler.ts @@ -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 } diff --git a/test/development/basic/gssp-ssr-change-reloading/test/index.test.ts b/test/development/basic/gssp-ssr-change-reloading/test/index.test.ts index 8d8cf6d63643..072943ed482c 100644 --- a/test/development/basic/gssp-ssr-change-reloading/test/index.test.ts +++ b/test/development/basic/gssp-ssr-change-reloading/test/index.test.ts @@ -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) => { @@ -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)