From 3f73d9aa8fb12845da811fe0a7b0cf4008b976cb Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 4 Oct 2023 12:31:33 -0600 Subject: [PATCH] refactor: replace match with definition for ensure, added dev handler interface + typings --- .../next/src/server/dev/hot-reloader-types.ts | 6 +- .../src/server/dev/hot-reloader-webpack.ts | 8 +- .../next/src/server/dev/next-dev-server.ts | 29 +++++-- .../src/server/dev/on-demand-entry-handler.ts | 79 ++++++++----------- packages/next/src/server/lib/dev-handlers.ts | 27 +++++++ packages/next/src/server/lib/router-server.ts | 33 +++----- .../src/server/lib/router-utils/setup-dev.ts | 9 ++- 7 files changed, 108 insertions(+), 83 deletions(-) create mode 100644 packages/next/src/server/lib/dev-handlers.ts diff --git a/packages/next/src/server/dev/hot-reloader-types.ts b/packages/next/src/server/dev/hot-reloader-types.ts index 191e4570697d0..922dc95452e0d 100644 --- a/packages/next/src/server/dev/hot-reloader-types.ts +++ b/packages/next/src/server/dev/hot-reloader-types.ts @@ -3,7 +3,7 @@ import type { UrlObject } from 'url' import type { Duplex } from 'stream' import type { webpack } from 'next/dist/compiled/webpack/webpack' import type getBaseWebpackConfig from '../../build/webpack-config' -import type { RouteMatch } from '../future/route-matches/route-match' +import type { RouteDefinition } from '../future/route-definitions/route-definition' import type { Project, Update as TurbopackUpdate } from '../../build/swc' import type { VersionInfo } from './parse-version-info' @@ -134,13 +134,13 @@ export interface NextJsHotReloaderInterface { page, clientOnly, appPaths, - match, + definition, isApp, }: { page: string clientOnly: boolean appPaths?: ReadonlyArray | null isApp?: boolean - match?: RouteMatch + definition: RouteDefinition | undefined }): Promise } diff --git a/packages/next/src/server/dev/hot-reloader-webpack.ts b/packages/next/src/server/dev/hot-reloader-webpack.ts index 20fa94826f3a1..48187503b3990 100644 --- a/packages/next/src/server/dev/hot-reloader-webpack.ts +++ b/packages/next/src/server/dev/hot-reloader-webpack.ts @@ -4,6 +4,7 @@ import type { Duplex } from 'stream' import type { Telemetry } from '../../telemetry/storage' import type { IncomingMessage, ServerResponse } from 'http' import type { UrlObject } from 'url' +import type { RouteDefinition } from '../future/route-definitions/route-definition' import { webpack, StringXor } from 'next/dist/compiled/webpack/webpack' import { getOverlayMiddleware } from 'next/dist/compiled/@next/react-dev-overlay/dist/middleware' @@ -60,7 +61,6 @@ import ws from 'next/dist/compiled/ws' import { existsSync, promises as fs } from 'fs' import { UnwrapPromise } from '../../lib/coalesced-function' import { getRegistry } from '../../lib/helpers/get-registry' -import { RouteMatch } from '../future/route-matches/route-match' import { parseVersionInfo, VersionInfo } from './parse-version-info' import { isAPIRoute } from '../../lib/is-api-route' import { getRouteLoaderEntry } from '../../build/webpack/loaders/next-route-loader' @@ -1466,14 +1466,14 @@ export default class HotReloader implements NextJsHotReloaderInterface { page, clientOnly, appPaths, - match, + definition, isApp, }: { page: string clientOnly: boolean appPaths?: ReadonlyArray | null isApp?: boolean - match?: RouteMatch + definition?: RouteDefinition }): Promise { // Make sure we don't re-build or dispose prebuilt pages if (page !== '/_error' && BLOCKED_PAGES.indexOf(page) !== -1) { @@ -1490,7 +1490,7 @@ export default class HotReloader implements NextJsHotReloaderInterface { page, clientOnly, appPaths, - match, + definition, isApp, }) } diff --git a/packages/next/src/server/dev/next-dev-server.ts b/packages/next/src/server/dev/next-dev-server.ts index 937e303461db4..8e95feca5b928 100644 --- a/packages/next/src/server/dev/next-dev-server.ts +++ b/packages/next/src/server/dev/next-dev-server.ts @@ -8,12 +8,13 @@ import type { UrlWithParsedQuery } from 'url' import type { BaseNextRequest, BaseNextResponse } from '../base-http' import type { FallbackMode, MiddlewareRoutingItem } from '../base-server' import type { FunctionComponent } from 'react' -import type { RouteMatch } from '../future/route-matches/route-match' +import type { RouteDefinition } from '../future/route-definitions/route-definition' import type { RouteMatcherManager } from '../future/route-matcher-managers/route-matcher-manager' import type { NextParsedUrlQuery, NextUrlWithParsedQuery, } from '../request-meta' +import type { DevHandlers } from '../lib/dev-handlers' import fs from 'fs' import { Worker } from 'next/dist/compiled/jest-worker' @@ -97,7 +98,17 @@ export default class DevServer extends Server { UnwrapPromise> > - private invokeDevMethod({ method, args }: { method: string; args: any[] }) { + private invokeDevMethod({ + method, + args, + }: { + method: M + /** + * This filters out the first `dir` argument from the method signature, as + * it's added by this method when invoking. + */ + args: DevHandlers[M] extends (_: any, ...rest: infer P) => any ? P : never + }) { return (global as any)._nextDevHandlers[method](this.dir, ...args) } @@ -190,7 +201,7 @@ export default class DevServer extends Server { const ensurer: RouteEnsurer = { ensure: async (match) => { await this.ensurePage({ - match, + definition: match.definition, page: match.definition.page, clientOnly: false, }) @@ -534,6 +545,7 @@ export default class DevServer extends Server { return this.ensurePage({ page: this.actualMiddlewareFile!, clientOnly: false, + definition: undefined, }) } @@ -543,6 +555,7 @@ export default class DevServer extends Server { (await this.ensurePage({ page: this.actualInstrumentationHookFile!, clientOnly: false, + definition: undefined, }) .then(() => true) .catch(() => false)) @@ -570,7 +583,12 @@ export default class DevServer extends Server { page: string appPaths: string[] | null }) { - return this.ensurePage({ page, appPaths, clientOnly: false }) + return this.ensurePage({ + page, + appPaths, + clientOnly: false, + definition: undefined, + }) } generateRoutes(_dev?: boolean) { @@ -723,7 +741,7 @@ export default class DevServer extends Server { page: string clientOnly: boolean appPaths?: ReadonlyArray | null - match?: RouteMatch + definition: RouteDefinition | undefined }): Promise { await this.invokeDevMethod({ method: 'ensurePage', @@ -759,6 +777,7 @@ export default class DevServer extends Server { page, appPaths, clientOnly: false, + definition: undefined, }) } diff --git a/packages/next/src/server/dev/on-demand-entry-handler.ts b/packages/next/src/server/dev/on-demand-entry-handler.ts index c5ef3a5802cef..0529aeb9d56f4 100644 --- a/packages/next/src/server/dev/on-demand-entry-handler.ts +++ b/packages/next/src/server/dev/on-demand-entry-handler.ts @@ -33,7 +33,6 @@ import { COMPILER_NAMES, RSC_MODULE_TYPES, } from '../../shared/lib/constants' -import { RouteMatch } from '../future/route-matches/route-match' import { HMR_ACTIONS_SENT_TO_BROWSER } from './hot-reloader-types' import HotReloader from './hot-reloader-webpack' import { isAppPageRouteDefinition } from '../future/route-definitions/app-page-route-definition' @@ -677,13 +676,13 @@ export function onDemandEntryHandler({ page, clientOnly, appPaths, - match, + definition, isApp, }: { page: string clientOnly: boolean appPaths: ReadonlyArray | null - match: Pick | undefined + definition: RouteDefinition | undefined isApp: boolean | undefined }): Promise { const stalledTime = 60 @@ -694,11 +693,11 @@ export function onDemandEntryHandler({ }, stalledTime * 1000) try { - let definition: Pick - if (match?.definition) { - definition = match.definition + let route: Pick + if (definition) { + route = definition } else { - definition = await findPagePathData( + route = await findPagePathData( rootDir, page, nextConfig.pageExtensions, @@ -707,18 +706,18 @@ export function onDemandEntryHandler({ ) } - const isInsideAppDir = !!appDir && definition.filename.startsWith(appDir) + const isInsideAppDir = !!appDir && route.filename.startsWith(appDir) if (typeof isApp === 'boolean' && isApp !== isInsideAppDir) { Error.stackTraceLimit = 15 throw new Error( `Ensure bailed, found path "${ - definition.page + route.page }" does not match ensure type (${isApp ? 'app' : 'pages'})` ) } - const pageBundleType = getPageBundleType(definition.bundlePath) + const pageBundleType = getPageBundleType(route.bundlePath) const addEntry = ( compilerType: CompilerNameValues ): { @@ -726,11 +725,7 @@ export function onDemandEntryHandler({ newEntry: boolean shouldInvalidate: boolean } => { - const entryKey = getEntryKey( - compilerType, - pageBundleType, - definition.page - ) + const entryKey = getEntryKey(compilerType, pageBundleType, route.page) if ( curEntries[entryKey] && // there can be an overlap in the entryKey for the instrumentation hook file and a page named the same @@ -758,9 +753,9 @@ export function onDemandEntryHandler({ curEntries[entryKey] = { type: EntryTypes.ENTRY, appPaths, - absolutePagePath: definition.filename, - request: definition.filename, - bundlePath: definition.bundlePath, + absolutePagePath: route.filename, + request: route.filename, + bundlePath: route.bundlePath, dispose: false, lastActiveTime: Date.now(), status: ADDED, @@ -774,7 +769,7 @@ export function onDemandEntryHandler({ const staticInfo = await getStaticInfoIncludingLayouts({ page, - pageFilePath: definition.filename, + pageFilePath: route.filename, isInsideAppDir, pageExtensions: nextConfig.pageExtensions, isDev: true, @@ -787,7 +782,7 @@ export function onDemandEntryHandler({ isInsideAppDir && staticInfo.rsc !== RSC_MODULE_TYPES.client runDependingOnPageType({ - page: definition.page, + page: route.page, pageRuntime: staticInfo.runtime, pageType: pageBundleType, onClient: () => { @@ -802,11 +797,11 @@ export function onDemandEntryHandler({ const edgeServerEntry = getEntryKey( COMPILER_NAMES.edgeServer, pageBundleType, - definition.page + route.page ) if ( curEntries[edgeServerEntry] && - !isInstrumentationHookFile(definition.page) + !isInstrumentationHookFile(route.page) ) { // Runtime switched from edge to server delete curEntries[edgeServerEntry] @@ -820,11 +815,11 @@ export function onDemandEntryHandler({ const serverEntry = getEntryKey( COMPILER_NAMES.server, pageBundleType, - definition.page + route.page ) if ( curEntries[serverEntry] && - !isInstrumentationHookFile(definition.page) + !isInstrumentationHookFile(route.page) ) { // Runtime switched from server to edge delete curEntries[serverEntry] @@ -839,9 +834,7 @@ export function onDemandEntryHandler({ const hasNewEntry = addedValues.some((entry) => entry.newEntry) if (hasNewEntry) { - reportTrigger( - !clientOnly && hasNewEntry ? `${definition.page}` : definition.page - ) + reportTrigger(!clientOnly && hasNewEntry ? `${route.page}` : route.page) } if (entriesThatShouldBeInvalidated.length > 0) { @@ -883,18 +876,12 @@ export function onDemandEntryHandler({ page: string clientOnly: boolean appPaths?: ReadonlyArray | null - match?: RouteMatch + definition?: RouteDefinition isApp?: boolean } // Make sure that we won't have multiple invalidations ongoing concurrently. - const batcher = Batcher.create< - Omit & { - definition?: RouteDefinition - }, - void, - string - >({ + const batcher = Batcher.create({ // The cache key here is composed of the elements that affect the // compilation, namely, the page, whether it's client only, and whether // it's an app page. This ensures that we don't have multiple compilations @@ -913,26 +900,28 @@ export function onDemandEntryHandler({ page, clientOnly, appPaths = null, - match, + definition, isApp, }: EnsurePageOptions) { // If the route is actually an app page route, then we should have access - // to the app route match, and therefore, the appPaths from it. - if ( - !appPaths && - match?.definition && - isAppPageRouteDefinition(match.definition) - ) { - appPaths = match.definition.appPaths + // to the app route definition, and therefore, the appPaths from it. + if (!appPaths && definition && isAppPageRouteDefinition(definition)) { + appPaths = definition.appPaths } // Wrap the invocation of the ensurePageImpl function in the pending // wrapper, which will ensure that we don't have multiple compilations // for the same page happening concurrently. return batcher.batch( - { page, clientOnly, appPaths, definition: match?.definition, isApp }, + { page, clientOnly, appPaths, definition, isApp }, async () => { - await ensurePageImpl({ page, clientOnly, appPaths, match, isApp }) + await ensurePageImpl({ + page, + clientOnly, + appPaths, + definition, + isApp, + }) } ) }, diff --git a/packages/next/src/server/lib/dev-handlers.ts b/packages/next/src/server/lib/dev-handlers.ts new file mode 100644 index 0000000000000..1060ef3f6e6b4 --- /dev/null +++ b/packages/next/src/server/lib/dev-handlers.ts @@ -0,0 +1,27 @@ +import type { IncomingMessage } from 'http' +import type { NextJsHotReloaderInterface } from '../dev/hot-reloader-types' + +/** + * The interface for the dev handlers global. + */ +export interface DevHandlers { + ensurePage( + dir: string, + ...args: Parameters + ): Promise + logErrorWithOriginalStack( + dir: string, + err: unknown, + type?: 'unhandledRejection' | 'uncaughtException' | 'warning' | 'app-dir' + ): Promise + getFallbackErrorComponents(dir: string): Promise + getCompilationError(dir: string, page: string): Promise + revalidate( + dir: string, + opts: { + urlPath: string + revalidateHeaders: IncomingMessage['headers'] + opts: any + } + ): Promise<{}> +} diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts index b16858a506458..c79c6e07150de 100644 --- a/packages/next/src/server/lib/router-server.ts +++ b/packages/next/src/server/lib/router-server.ts @@ -1,5 +1,3 @@ -import type { IncomingMessage } from 'http' - // this must come first as it includes require hooks import type { WorkerRequestHandler, WorkerUpgradeHandler } from './types' @@ -35,7 +33,7 @@ import { PHASE_DEVELOPMENT_SERVER, PERMANENT_REDIRECT_STATUS, } from '../../shared/lib/constants' -import type { NextJsHotReloaderInterface } from '../dev/hot-reloader-types' +import { DevHandlers } from './dev-handlers' const debug = setupDebug('next:router-server:main') @@ -130,20 +128,16 @@ export async function initialize(opts: { }) devInstances[opts.dir] = devInstance ;(global as any)._nextDevHandlers = { - async ensurePage( - dir: string, - match: Parameters[0] - ) { + async ensurePage(dir, definition) { const curDevInstance = devInstances[dir] // TODO: remove after ensure is pulled out of server - return await curDevInstance?.hotReloader.ensurePage(match) + return await curDevInstance?.hotReloader.ensurePage(definition) }, - async logErrorWithOriginalStack(dir: string, ...args: any[]) { + async logErrorWithOriginalStack(dir, ...args) { const curDevInstance = devInstances[dir] - // @ts-ignore return await curDevInstance?.logErrorWithOriginalStack(...args) }, - async getFallbackErrorComponents(dir: string) { + async getFallbackErrorComponents(dir) { const curDevInstance = devInstances[dir] await curDevInstance.hotReloader.buildFallbackError() // Build the error page to ensure the fallback is built too. @@ -151,9 +145,10 @@ export async function initialize(opts: { await curDevInstance.hotReloader.ensurePage({ page: '/_error', clientOnly: false, + definition: undefined, }) }, - async getCompilationError(dir: string, page: string) { + async getCompilationError(dir, page) { const curDevInstance = devInstances[dir] const errors = await curDevInstance?.hotReloader?.getCompilationErrors( page @@ -164,16 +159,8 @@ export async function initialize(opts: { return errors[0] }, async revalidate( - dir: string, - { - urlPath, - revalidateHeaders, - opts: revalidateOpts, - }: { - urlPath: string - revalidateHeaders: IncomingMessage['headers'] - opts: any - } + dir, + { urlPath, revalidateHeaders, opts: revalidateOpts } ) { const mocked = createRequestResponseMocks({ url: urlPath, @@ -195,7 +182,7 @@ export async function initialize(opts: { } return {} }, - } as any + } satisfies DevHandlers } renderServer.instance = diff --git a/packages/next/src/server/lib/router-utils/setup-dev.ts b/packages/next/src/server/lib/router-utils/setup-dev.ts index f97e14c15178b..98575e8526e0a 100644 --- a/packages/next/src/server/lib/router-utils/setup-dev.ts +++ b/packages/next/src/server/lib/router-utils/setup-dev.ts @@ -1065,6 +1065,7 @@ async function startWatcher(opts: SetupOpts) { .ensurePage({ page: decodedPagePath, clientOnly: false, + definition: undefined, }) .catch(console.error) } @@ -1166,10 +1167,10 @@ async function startWatcher(opts: SetupOpts) { // Unused parameters // clientOnly, // appPaths, - match, + definition, isApp, }) { - let page = match?.definition?.pathname ?? inputPage + let page = definition?.pathname ?? inputPage if (page === '/_error') { if (globalEntries.app) { @@ -1218,7 +1219,7 @@ async function startWatcher(opts: SetupOpts) { curEntries.get(page) ?? curEntries.get( normalizeAppPath( - normalizeMetadataRoute(match?.definition?.page ?? inputPage) + normalizeMetadataRoute(definition?.page ?? inputPage) ) ) @@ -1457,6 +1458,7 @@ async function startWatcher(opts: SetupOpts) { clientOnly: false, page: item.itemPath, isApp: item.type === 'appFile', + definition: undefined, }) } }) @@ -2236,6 +2238,7 @@ async function startWatcher(opts: SetupOpts) { return hotReloader.ensurePage({ page: serverFields.actualMiddlewareFile, clientOnly: false, + definition: undefined, }) }, }