Skip to content

Commit

Permalink
perf(turbopack): only build last parallel route (#67588)
Browse files Browse the repository at this point in the history
### What?

This partially reverts #67222, it's unnecessary to build all parallel
routes and also seems to have a higher overhead than I thought.
  • Loading branch information
ForsakenHarmony authored Jul 10, 2024
1 parent e15549c commit 5173356
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 55 deletions.
114 changes: 60 additions & 54 deletions packages/next/src/server/dev/hot-reloader-turbopack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = !!(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@
"target": "ES2017"
},
"include": ["next-env.d.ts", ".next/types/**/*.ts", "**/*.ts", "**/*.tsx"],
"exclude": ["node_modules"]
"exclude": ["node_modules", "**/*.test.ts"]
}

0 comments on commit 5173356

Please sign in to comment.