From 4df48390dbd170fe3ee61a0f9f960ffb167d4a9a Mon Sep 17 00:00:00 2001 From: hrmny <8845940+ForsakenHarmony@users.noreply.github.com> Date: Tue, 2 Jul 2024 15:12:33 +0200 Subject: [PATCH] fix(turbopack): build all parallel routes to make sure HMR works (#67222) ### What? Parallel routes build in a weird and render in a weird way. To render next.js always uses the last parallel route (alphabetically I think) as that's most likely to be the root. Building works in the opposite order, it goes from the first to the last. This is fine for the first render, but after that the renderer will only check if the file for the last parallel route has a bundle on disk and never request it to be updated. 1. match on routes 2. build match 3. check if the last parallel route bundle exists on disk - if it doesn't so go back to step 2 with the next match - the actual match gets thrown away 4. render with the bundle for the last parallel route The condition in step 3 will always be true after an update, because it was built for a previous request To fix this, turbopack will now emit all parallel routes that match a path every time one of them gets requested. Closes PACK-3078 Fixes #65836 --- .../src/server/dev/hot-reloader-turbopack.ts | 102 ++++++++++-------- 1 file changed, 58 insertions(+), 44 deletions(-) diff --git a/packages/next/src/server/dev/hot-reloader-turbopack.ts b/packages/next/src/server/dev/hot-reloader-turbopack.ts index 49761d3999..671f4a76de 100644 --- a/packages/next/src/server/dev/hot-reloader-turbopack.ts +++ b/packages/next/src/server/dev/hot-reloader-turbopack.ts @@ -80,6 +80,7 @@ import { } from './turbopack/entry-key' import { FAST_REFRESH_RUNTIME_RELOAD } from './messages' import { generateEncryptionKeyBase64 } from '../app-render/encryption-utils' +import { isAppPageRouteDefinition } from '../route-definitions/app-page-route-definition' const wsServer = new ws.Server({ noServer: true }) const isTestMode = !!( @@ -762,7 +763,7 @@ export async function createHotReloaderTurbopack( page: inputPage, // Unused parameters // clientOnly, - // appPaths, + appPaths, definition, isApp, url: requestUrl, @@ -784,6 +785,14 @@ export async function createHotReloaderTurbopack( const page = routeDef.page const pathname = definition?.pathname ?? inputPage + let pages = appPaths ?? [page] + + // If the route is actually an app page route, then we should have access + // to the app route definition, and therefore, the appPaths from it. + if (!appPaths && definition && isAppPageRouteDefinition(definition)) { + pages = definition.appPaths + } + if (page === '/_error') { let finishBuilding = startBuilding(pathname, requestUrl, false) try { @@ -811,54 +820,59 @@ export async function createHotReloaderTurbopack( await currentEntriesHandling const isInsideAppDir = routeDef.bundlePath.startsWith('app/') - const normalizedAppPage = normalizedPageToTurbopackStructureRoute( - page, - extname(routeDef.filename) - ) - - const route = isInsideAppDir - ? currentEntrypoints.app.get(normalizedAppPage) - : currentEntrypoints.page.get(page) - - if (!route) { - // TODO: why is this entry missing in turbopack? - if (page === '/middleware') return - if (page === '/src/middleware') return - if (page === '/instrumentation') return - if (page === '/src/instrumentation') return - - throw new PageNotFoundError(`route not found ${page}`) - } - - // We don't throw on ensureOpts.isApp === true for page-api - // since this can happen when app pages make - // api requests to page API routes. - if (isApp && route.type === 'page') { - throw new Error(`mis-matched route type: isApp && page for ${page}`) - } const finishBuilding = startBuilding(pathname, requestUrl, false) try { - await handleRouteType({ - dev: true, - page, - pathname, - route, - currentEntryIssues, - entrypoints: currentEntrypoints, - manifestLoader, - readyIds, - rewrites: opts.fsChecker.rewrites, - logErrors: true, + // we need to build all parallel routes, so we loop over them here - hooks: { - subscribeToChanges, - handleWrittenEndpoint: (id, result) => { - clearRequireCache(id, result) - assetMapper.setPathsForKey(id, result.clientPaths) + /* eslint-disable-next-line @typescript-eslint/no-shadow -- intentionally shadowed*/ + for (const page of pages) { + const normalizedAppPage = normalizedPageToTurbopackStructureRoute( + page, + extname(routeDef.filename) + ) + const route = isInsideAppDir + ? currentEntrypoints.app.get(normalizedAppPage) + : currentEntrypoints.page.get(page) + + if (!route) { + // TODO: why is this entry missing in turbopack? + if (page === '/middleware') return + if (page === '/src/middleware') return + if (page === '/instrumentation') return + if (page === '/src/instrumentation') return + + throw new PageNotFoundError(`route not found ${page}`) + } + + // We don't throw on ensureOpts.isApp === true for page-api + // since this can happen when app pages make + // api requests to page API routes. + if (isApp && route.type === 'page') { + throw new Error(`mis-matched route type: isApp && page for ${page}`) + } + + await handleRouteType({ + dev: true, + page, + pathname, + route, + currentEntryIssues, + entrypoints: currentEntrypoints, + manifestLoader, + readyIds, + rewrites: opts.fsChecker.rewrites, + logErrors: true, + + hooks: { + subscribeToChanges, + handleWrittenEndpoint: (id, result) => { + clearRequireCache(id, result) + assetMapper.setPathsForKey(id, result.clientPaths) + }, }, - }, - }) + }) + } } finally { finishBuilding() }