From 6a4a0c0e07bc586099af7a968d647a7a83bb24b6 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Wed, 25 Sep 2024 10:52:12 -0700 Subject: [PATCH] refactor: added more strict app segment config parsing --- .../next/src/build/app-segment-config.test.ts | 110 +++++++++ packages/next/src/build/app-segment-config.ts | 74 +++++++ packages/next/src/build/index.ts | 5 +- packages/next/src/build/utils.ts | 208 +++++++++--------- .../src/server/dev/static-paths-worker.ts | 4 +- .../server/route-modules/app-route/module.ts | 7 +- packages/next/types/compiled.d.ts | 6 + 7 files changed, 308 insertions(+), 106 deletions(-) create mode 100644 packages/next/src/build/app-segment-config.test.ts create mode 100644 packages/next/src/build/app-segment-config.ts diff --git a/packages/next/src/build/app-segment-config.test.ts b/packages/next/src/build/app-segment-config.test.ts new file mode 100644 index 0000000000000..2fee918effb07 --- /dev/null +++ b/packages/next/src/build/app-segment-config.test.ts @@ -0,0 +1,110 @@ +import { AppSegmentConfigSchema } from './app-segment-config' + +describe('AppConfigSchema', () => { + it('should only support zero, a positive number or false for revalidate', () => { + const valid = [0, 1, 100, false] + + for (const value of valid) { + expect( + AppSegmentConfigSchema.safeParse({ revalidate: value }).success + ).toBe(true) + } + + const invalid = [-1, -100, true] + + for (const value of invalid) { + expect( + AppSegmentConfigSchema.safeParse({ revalidate: value }).success + ).toBe(false) + } + }) + + it('should support an empty config', () => { + expect(AppSegmentConfigSchema.safeParse({}).success).toBe(true) + }) + + it('should support a boolean for dynamicParams', () => { + expect( + AppSegmentConfigSchema.safeParse({ dynamicParams: true }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ dynamicParams: false }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ dynamicParams: 'foo' }).success + ).toBe(false) + }) + + it('should support "auto" | "force-dynamic" | "error" | "force-static" for dynamic', () => { + expect(AppSegmentConfigSchema.safeParse({ dynamic: 'auto' }).success).toBe( + true + ) + expect( + AppSegmentConfigSchema.safeParse({ dynamic: 'force-dynamic' }).success + ).toBe(true) + expect(AppSegmentConfigSchema.safeParse({ dynamic: 'error' }).success).toBe( + true + ) + expect( + AppSegmentConfigSchema.safeParse({ dynamic: 'force-static' }).success + ).toBe(true) + }) + + it('should support "edge" | "nodejs" for runtime', () => { + expect(AppSegmentConfigSchema.safeParse({ runtime: 'edge' }).success).toBe( + true + ) + expect( + AppSegmentConfigSchema.safeParse({ runtime: 'nodejs' }).success + ).toBe(true) + expect(AppSegmentConfigSchema.safeParse({ runtime: 'foo' }).success).toBe( + false + ) + }) + + it('should support a positive number or zero for maxDuration', () => { + expect(AppSegmentConfigSchema.safeParse({ maxDuration: 0 }).success).toBe( + true + ) + expect(AppSegmentConfigSchema.safeParse({ maxDuration: 100 }).success).toBe( + true + ) + expect(AppSegmentConfigSchema.safeParse({ maxDuration: -1 }).success).toBe( + false + ) + }) + + it('should support "force-cache" | "only-cache" for fetchCache', () => { + expect( + AppSegmentConfigSchema.safeParse({ fetchCache: 'force-cache' }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ fetchCache: 'only-cache' }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ fetchCache: 'foo' }).success + ).toBe(false) + }) + + it('should support a string or an array of strings for preferredRegion', () => { + expect( + AppSegmentConfigSchema.safeParse({ preferredRegion: 'foo' }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ preferredRegion: ['foo', 'bar'] }) + .success + ).toBe(true) + }) + + it('should support a boolean for experimental_ppr', () => { + expect( + AppSegmentConfigSchema.safeParse({ experimental_ppr: true }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ experimental_ppr: false }).success + ).toBe(true) + expect( + AppSegmentConfigSchema.safeParse({ experimental_ppr: 'foo' }).success + ).toBe(false) + }) +}) diff --git a/packages/next/src/build/app-segment-config.ts b/packages/next/src/build/app-segment-config.ts new file mode 100644 index 0000000000000..4cd75afb0b5e2 --- /dev/null +++ b/packages/next/src/build/app-segment-config.ts @@ -0,0 +1,74 @@ +import { z } from 'next/dist/compiled/zod' + +/** + * The schema for the dynamic behavior of a page. + */ +export const AppSegmentConfigDynamicSchema = z.enum([ + 'auto', + 'error', + 'force-static', + 'force-dynamic', +]) + +/** + * The dynamic behavior of the page. + */ +export type AppSegmentConfigDynamic = z.infer< + typeof AppSegmentConfigDynamicSchema +> + +/** + * The schema for configuration for a page. + */ +export const AppSegmentConfigSchema = z.object({ + /** + * The number of seconds to revalidate the page or false to disable revalidation. + */ + revalidate: z + .union([z.number().int().nonnegative(), z.literal(false)]) + .optional(), + + /** + * Whether the page supports dynamic parameters. + */ + dynamicParams: z.boolean().optional(), + + /** + * The dynamic behavior of the page. + */ + dynamic: AppSegmentConfigDynamicSchema.optional(), + + /** + * The caching behavior of the page. + */ + fetchCache: z.enum(['force-cache', 'only-cache']).optional(), + + /** + * The preferred region for the page. + */ + preferredRegion: z.union([z.string(), z.array(z.string())]).optional(), + + /** + * Whether the page supports partial prerendering. When true, the page will be + * served using partial prerendering. This setting will only take affect if + * it's enabled via the `experimental.ppr = "incremental"` option. + */ + experimental_ppr: z.boolean().optional(), + + /** + * The runtime to use for the page. + */ + runtime: z.enum(['edge', 'nodejs']).optional(), + + /** + * The maximum duration for the page in seconds. + */ + maxDuration: z.number().int().nonnegative().optional(), +}) + +/** + * The configuration for a page. + */ +export type AppSegmentConfig = z.infer + +export const AppSegmentConfigSchemaKeys = AppSegmentConfigSchema.keyof().options diff --git a/packages/next/src/build/index.ts b/packages/next/src/build/index.ts index d5a8508b4d2cd..6c071dd5523b1 100644 --- a/packages/next/src/build/index.ts +++ b/packages/next/src/build/index.ts @@ -130,7 +130,8 @@ import { collectMeta, // getSupportedBrowsers, } from './utils' -import type { PageInfo, PageInfos, AppConfig, PrerenderedRoute } from './utils' +import type { PageInfo, PageInfos, PrerenderedRoute } from './utils' +import type { AppSegmentConfig } from './app-segment-config' import { writeBuildId } from './write-build-id' import { normalizeLocalePath } from '../shared/lib/i18n/normalize-locale-path' import isError from '../lib/is-error' @@ -1806,7 +1807,7 @@ export default async function build( const staticPaths = new Map() const appNormalizedPaths = new Map() const fallbackModes = new Map() - const appDefaultConfigs = new Map() + const appDefaultConfigs = new Map() const pageInfos: PageInfos = new Map() let pagesManifest = await readManifest(pagesManifestPath) const buildManifest = await readManifest(buildManifestPath) diff --git a/packages/next/src/build/utils.ts b/packages/next/src/build/utils.ts index c27b2f78e5d12..77bf2f85a0e32 100644 --- a/packages/next/src/build/utils.ts +++ b/packages/next/src/build/utils.ts @@ -97,6 +97,12 @@ import { } from '../lib/fallback' import { getParamKeys } from '../server/request/fallback-params' import type { OutgoingHttpHeaders } from 'http' +import { + AppSegmentConfigSchema, + type AppSegmentConfig, +} from './app-segment-config' +import { DEFAULT_SEGMENT_KEY, PAGE_SEGMENT_KEY } from '../shared/lib/segment' +import type { AppDirModules } from './webpack/loaders/next-app-loader' export type ROUTER_TYPE = 'pages' | 'app' @@ -1200,73 +1206,59 @@ export async function buildStaticPaths({ } } -export type AppConfigDynamic = - | 'auto' - | 'error' - | 'force-static' - | 'force-dynamic' - -export type AppConfig = { - revalidate?: number | false - dynamicParams?: true | false - dynamic?: AppConfigDynamic - fetchCache?: 'force-cache' | 'only-cache' - preferredRegion?: string - - /** - * When true, the page will be served using partial prerendering. - * This setting will only take affect if it's enabled via - * the `experimental.ppr = "incremental"` option. - */ - experimental_ppr?: boolean -} - type GenerateStaticParams = (options: { params?: Params }) => Promise -type GenerateParamsResult = { - config?: AppConfig - isDynamicSegment?: boolean +export type GenerateParamsResult = { + segment?: string segmentPath: string + config: AppSegmentConfig | undefined + isDynamicSegment?: boolean generateStaticParams?: GenerateStaticParams - isLayout?: boolean } -export type GenerateParamsResults = GenerateParamsResult[] - -const collectAppConfig = ( - mod: Partial | undefined -): AppConfig | undefined => { - let hasConfig = false - const config: AppConfig = {} +const RESERVED_SEGMENT_KEYS = [PAGE_SEGMENT_KEY, DEFAULT_SEGMENT_KEY] - if (typeof mod?.revalidate !== 'undefined') { - config.revalidate = mod.revalidate - hasConfig = true - } - if (typeof mod?.dynamicParams !== 'undefined') { - config.dynamicParams = mod.dynamicParams - hasConfig = true - } - if (typeof mod?.dynamic !== 'undefined') { - config.dynamic = mod.dynamic - hasConfig = true - } - if (typeof mod?.fetchCache !== 'undefined') { - config.fetchCache = mod.fetchCache - hasConfig = true - } - if (typeof mod?.preferredRegion !== 'undefined') { - config.preferredRegion = mod.preferredRegion - hasConfig = true +/** + * Get the module from the components. This only returns the module if it is + * a page or layout. + * + * @param components - The components to get the module from. + * @returns The module and the type of module. + */ +async function getPageOrLayoutModule( + components: AppDirModules +): Promise<{ mod?: any; type?: keyof AppDirModules }> { + if (typeof components.layout !== 'undefined') { + return { mod: await components.layout[0]?.(), type: 'layout' } } - if (typeof mod?.experimental_ppr !== 'undefined') { - config.experimental_ppr = mod.experimental_ppr - hasConfig = true + + if (typeof components.page !== 'undefined') { + return { mod: await components.page[0]?.(), type: 'page' } } - if (!hasConfig) return undefined + // Try to get the first module that is not undefined. + const type = Object.keys(components).find((key) => { + return typeof components[key as keyof AppDirModules] !== 'undefined' + }) as keyof AppDirModules | undefined - return config + return { type } +} + +const ModuleName: Record = { + layout: 'Layout', + page: 'Page', + template: 'Template', + error: 'Error', + loading: 'Loading', + 'not-found': 'Not Found', + metadata: 'Metadata', + defaultPage: 'Default Page', +} + +function getModuleName(type: keyof AppDirModules | undefined) { + if (!type) return 'Component' + + return ModuleName[type] } /** @@ -1276,68 +1268,82 @@ const collectAppConfig = ( * @returns the generate parameters for each segment */ export async function collectGenerateParams(tree: LoaderTree) { - const generateParams: GenerateParamsResults = [] + const generateParams: GenerateParamsResult[] = [] + const staticGenerationGenerateParams: GenerateParamsResult[] = [] const parentSegments: string[] = [] let currentLoaderTree = tree while (currentLoaderTree) { - const [ - // TODO: check if this is ever undefined - page = '', - parallelRoutes, - components, - ] = currentLoaderTree - - // If the segment doesn't have any components, then skip it. + const [segment, parallelRoutes, components] = currentLoaderTree if (!components) continue - const isLayout = !!components.layout - const mod = await (isLayout - ? components.layout?.[0]?.() - : components.page?.[0]?.()) - - if (page) { - parentSegments.push(page) + const isReservedSegmentKey = RESERVED_SEGMENT_KEYS.includes(segment) + if (segment && !isReservedSegmentKey) { + parentSegments.push(segment) } - const config = mod ? collectAppConfig(mod) : undefined - const isClientComponent = isClientReference(mod) + const { mod, type } = await getPageOrLayoutModule(components) - const isDynamicSegment = /^\[.+\]$/.test(page) + const isClientComponent = mod && isClientReference(mod) + const isDynamicSegment = /^\[.+\]$/.test(segment) + const isParentDynamicSegment = + generateParams.length > 0 && + generateParams[generateParams.length - 1].isDynamicSegment - const { generateStaticParams } = mod || {} - - if (isDynamicSegment && isClientComponent && generateStaticParams) { - throw new Error( - `Page "${page}" cannot export "generateStaticParams()" because it is a client component` - ) + const result: GenerateParamsResult = { + segment, + config: undefined, + isDynamicSegment, + segmentPath: `/${parentSegments.join('/')}`, } - const segmentPath = `/${parentSegments.join('/')}${ - page && parentSegments.length > 0 ? '/' : '' - }${page}` + const segmentAppFilePath = `app/${[...parentSegments, type].join('/')}` - const result: GenerateParamsResult = { - isLayout, - isDynamicSegment, - segmentPath, - config, - generateStaticParams: !isClientComponent - ? generateStaticParams - : undefined, + if (!isClientComponent && mod) { + const { generateStaticParams } = mod + if (typeof generateStaticParams === 'function') { + result.generateStaticParams = generateStaticParams + } + + const config = AppSegmentConfigSchema.safeParse(mod) + if (config.success) { + result.config = config.data + + if ( + typeof result.config.dynamicParams === 'boolean' && + !isDynamicSegment && + (!isReservedSegmentKey || !isParentDynamicSegment) + ) { + throw new Error( + `${getModuleName(type)} "${segmentAppFilePath}" cannot export "dynamicParams" because it is not a dynamic segment` + ) + } + + if ( + result.config.dynamicParams === false && + !result.generateStaticParams + ) { + throw new Error( + `${getModuleName(type)} "${segmentAppFilePath}" must export "generateStaticParams" because it exports "dynamicParams: false"` + ) + } + } } + // Add the result to the generate params list. + generateParams.push(result) + // If the configuration contributes to the static generation, then add it // to the list. if (result.config || result.generateStaticParams || isDynamicSegment) { - generateParams.push(result) + staticGenerationGenerateParams.push(result) } // Use this route's parallel route children as the next segment. currentLoaderTree = parallelRoutes.children } - return generateParams + return staticGenerationGenerateParams } export type PartialStaticPathsResult = { @@ -1363,7 +1369,7 @@ export async function buildAppStaticPaths({ dir: string page: string configFileName: string - generateParams: GenerateParamsResults + generateParams: GenerateParamsResult[] distDir: string isrFlushToDisk?: boolean fetchCacheKeyPrefix?: string @@ -1565,7 +1571,7 @@ type PageIsStaticResult = { isNextImageImported?: boolean traceIncludes?: string[] traceExcludes?: string[] - appConfig?: AppConfig + appConfig?: AppSegmentConfig } export async function isPageStatic({ @@ -1622,7 +1628,7 @@ export async function isPageStatic({ let componentsResult: LoadComponentsReturnType let prerenderedRoutes: PrerenderedRoute[] | undefined let prerenderFallbackMode: FallbackMode | undefined - let appConfig: AppConfig = {} + let appConfig: AppSegmentConfig = {} let isClientComponent: boolean = false const pathIsEdgeRuntime = isEdgeRuntime(pageRuntime) @@ -1678,7 +1684,7 @@ export async function isPageStatic({ const { tree } = ComponentMod - const generateParams: GenerateParamsResults = + const generateParams: GenerateParamsResult[] = routeModule && isAppRouteRouteModule(routeModule) ? [ { @@ -1694,6 +1700,8 @@ export async function isPageStatic({ ] : await collectGenerateParams(tree) + // console.log('generateParams', generateParams) + appConfig = reduceAppConfig(generateParams) if (appConfig.dynamic === 'force-static' && pathIsEdgeRuntime) { @@ -1827,7 +1835,7 @@ export async function isPageStatic({ } type ReducedAppConfig = Pick< - AppConfig, + AppSegmentConfig, | 'dynamic' | 'fetchCache' | 'preferredRegion' @@ -1843,7 +1851,7 @@ type ReducedAppConfig = Pick< * @returns the reduced app config */ export function reduceAppConfig( - segments: GenerateParamsResults + segments: GenerateParamsResult[] ): ReducedAppConfig { const config: ReducedAppConfig = {} diff --git a/packages/next/src/server/dev/static-paths-worker.ts b/packages/next/src/server/dev/static-paths-worker.ts index dad3333f19dc3..30843986c0d14 100644 --- a/packages/next/src/server/dev/static-paths-worker.ts +++ b/packages/next/src/server/dev/static-paths-worker.ts @@ -10,7 +10,7 @@ import { reduceAppConfig, } from '../../build/utils' import type { - GenerateParamsResults, + GenerateParamsResult, PartialStaticPathsResult, } from '../../build/utils' import { loadComponents } from '../load-components' @@ -93,7 +93,7 @@ export async function loadStaticPaths({ if (isAppPath) { const { routeModule } = components - const generateParams: GenerateParamsResults = + const generateParams: GenerateParamsResult[] = routeModule && isAppRouteRouteModule(routeModule) ? [ { diff --git a/packages/next/src/server/route-modules/app-route/module.ts b/packages/next/src/server/route-modules/app-route/module.ts index 9112f8b5c1142..08d14c6c7ecf1 100644 --- a/packages/next/src/server/route-modules/app-route/module.ts +++ b/packages/next/src/server/route-modules/app-route/module.ts @@ -1,6 +1,6 @@ import type { NextConfig } from '../../config-shared' import type { AppRouteRouteDefinition } from '../../route-definitions/app-route-route-definition' -import type { AppConfig } from '../../../build/utils' +import type { AppSegmentConfig } from '../../../build/app-segment-config' import type { NextRequest } from '../../web/spec-extension/request' import type { PrerenderManifest } from '../../../build' import type { NextURL } from '../../web/next-url' @@ -122,7 +122,10 @@ export type AppRouteHandlers = { * routes. This contains all the user generated code. */ export type AppRouteUserlandModule = AppRouteHandlers & - Pick & { + Pick< + AppSegmentConfig, + 'dynamic' | 'revalidate' | 'dynamicParams' | 'fetchCache' + > & { // TODO: (wyattjoh) create a type for this generateStaticParams?: any } diff --git a/packages/next/types/compiled.d.ts b/packages/next/types/compiled.d.ts index 42038c4715284..b0d9f7c8e77f1 100644 --- a/packages/next/types/compiled.d.ts +++ b/packages/next/types/compiled.d.ts @@ -54,6 +54,12 @@ declare module 'next/dist/compiled/jest-worker' { } } +declare module 'next/dist/compiled/zod' { + // eslint-disable-next-line import/no-extraneous-dependencies + import * as m from 'zod' + export = m +} + declare module 'next/dist/compiled/amphtml-validator' { export type ValidationError = any }