From 517335673dca9b0542f0f805c2944b3c76a660bd Mon Sep 17 00:00:00 2001 From: hrmny <8845940+ForsakenHarmony@users.noreply.github.com> Date: Wed, 10 Jul 2024 16:46:53 +0200 Subject: [PATCH] perf(turbopack): only build last parallel route (#67588) ### What? This partially reverts #67222, it's unnecessary to build all parallel routes and also seems to have a higher overhead than I thought. --- .../src/server/dev/hot-reloader-turbopack.ts | 114 +++++++++--------- .../tsconfig.json | 2 +- 2 files changed, 61 insertions(+), 55 deletions(-) diff --git a/packages/next/src/server/dev/hot-reloader-turbopack.ts b/packages/next/src/server/dev/hot-reloader-turbopack.ts index 70267f8550eab..50cf91057129f 100644 --- a/packages/next/src/server/dev/hot-reloader-turbopack.ts +++ b/packages/next/src/server/dev/hot-reloader-turbopack.ts @@ -81,6 +81,7 @@ import { import { FAST_REFRESH_RUNTIME_RELOAD } from './messages' import { generateEncryptionKeyBase64 } from '../app-render/encryption-utils' import { isAppPageRouteDefinition } from '../route-definitions/app-page-route-definition' +import { normalizeAppPath } from '../../shared/lib/router/utils/app-paths' const wsServer = new ws.Server({ noServer: true }) const isTestMode = !!( @@ -784,17 +785,27 @@ export async function createHotReloaderTurbopack( opts.appDir )) - 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 + appPaths = definition.appPaths } + let page = routeDef.page + if (appPaths) { + const normalizedPage = normalizeAppPath(page) + + // filter out paths that are not exact matches (e.g. catchall) + const matchingAppPaths = appPaths.filter( + (path) => normalizeAppPath(path) === normalizedPage + ) + + // the last item in the array is the root page, if there are parallel routes + page = matchingAppPaths[matchingAppPaths.length - 1] + } + + const pathname = definition?.pathname ?? inputPage + if (page === '/_error') { let finishBuilding = startBuilding(pathname, requestUrl, false) try { @@ -823,60 +834,55 @@ export async function createHotReloaderTurbopack( await currentEntriesHandling const isInsideAppDir = routeDef.bundlePath.startsWith('app/') + const normalizedAppPage = normalizedPageToTurbopackStructureRoute( + page, + extname(routeDef.filename) + ) - const finishBuilding = startBuilding(pathname, requestUrl, false) - try { - // we need to build all parallel routes, so we loop over them here + const route = isInsideAppDir + ? currentEntrypoints.app.get(normalizedAppPage) + : currentEntrypoints.page.get(page) - /* 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}`) - } + 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 - // 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}`) - } + throw new PageNotFoundError(`route not found ${page}`) + } - await handleRouteType({ - dev: true, - page, - pathname, - route, - currentEntryIssues, - entrypoints: currentEntrypoints, - manifestLoader, - readyIds, - devRewrites: opts.fsChecker.rewrites, - productionRewrites: undefined, - logErrors: true, + // 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}`) + } - hooks: { - subscribeToChanges, - handleWrittenEndpoint: (id, result) => { - clearRequireCache(id, result) - assetMapper.setPathsForKey(id, result.clientPaths) - }, + const finishBuilding = startBuilding(pathname, requestUrl, false) + try { + await handleRouteType({ + dev: true, + page, + pathname, + route, + currentEntryIssues, + entrypoints: currentEntrypoints, + manifestLoader, + readyIds, + devRewrites: opts.fsChecker.rewrites, + productionRewrites: undefined, + logErrors: true, + + hooks: { + subscribeToChanges, + handleWrittenEndpoint: (id, result) => { + clearRequireCache(id, result) + assetMapper.setPathsForKey(id, result.clientPaths) }, - }) - } + }, + }) } finally { finishBuilding() } diff --git a/test/e2e/app-dir/parallel-routes-and-interception/tsconfig.json b/test/e2e/app-dir/parallel-routes-and-interception/tsconfig.json index 2eef1368c65e7..d218731c4c6f7 100644 --- a/test/e2e/app-dir/parallel-routes-and-interception/tsconfig.json +++ b/test/e2e/app-dir/parallel-routes-and-interception/tsconfig.json @@ -21,5 +21,5 @@ "target": "ES2017" }, "include": ["next-env.d.ts", ".next/types/**/*.ts", "**/*.ts", "**/*.tsx"], - "exclude": ["node_modules"] + "exclude": ["node_modules", "**/*.test.ts"] }