feat: new routing system build time changes (#153)

* feat: utility functions for route path names * feat: format path names + check invalid generated functions after * feat: adjust middleware manifest to support new route system fixes * feat: process vercel config and build output * feat: connect new build time to worker script * chore: changeset * chore: add issue reference to comments * feat: `VercelBuildOutputItem` type Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: refine comment Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: refine comment Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: apply suggestions from code review Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: address suggestions * chore: make prettier happy * test(fs): readPathsRecursively * test: routing utils * fix: invalid rsc functions with valid squashed non-rsc functions * chore: move part of comment * feat: use `relative` for static asset name resolution * feat: add build output config overrides to build output map * chore: change type to be of override instead of static * fix: wrong type in test * fix: derp, fix type error * chore: apply suggestions from code review Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: prettier * fix: add back `/page` checking for older nextjs versions middleware manifests * chore: apply suggestions from code review Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> * chore: prettier * feat: improve vercel types once more * Update src/buildApplication/middlewareManifest.ts * chore: apply suggestions from code review Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com> --------- Co-authored-by: Dario Piotrowicz <dario.piotrowicz@gmail.com>
cloudflare · Apr 12, 2023 · 3387ac9 · 3387ac9
1 parent 9834066
commit 3387ac9
Show file tree

Hide file tree

Showing 23 changed files with 1,025 additions and 144 deletions.
diff --git a/.changeset/spotty-cows-add.md b/.changeset/spotty-cows-add.md
@@ -0,0 +1,5 @@
+---
+'@cloudflare/next-on-pages': minor
+---
+
+New routing system build time processing + integration with worker script.
diff --git a/src/buildApplication/buildApplication.ts b/src/buildApplication/buildApplication.ts
@@ -15,6 +15,10 @@ import {
 } from './buildVercelOutput';
 import { buildMetadataFiles } from './buildMetadataFiles';
 import { validateDir } from '../utils';
+import {
+	getVercelStaticAssets,
+	processVercelOutput,
+} from './processVercelOutput';
 
 /**
  * Builds the _worker.js with static assets implementing the Next.js application
@@ -62,9 +66,7 @@ async function prepareAndBuildWorker(
 		cliLog(`Using basePath ${nextJsConfigs.basePath}`);
 	}
 
-	const functionsDir = resolve(
-		`.vercel/output/functions${nextJsConfigs.basePath ?? ''}`
-	);
+	const functionsDir = resolve(`.vercel/output/functions`);
 	if (!(await validateDir(functionsDir))) {
 		cliLog('No functions detected.');
 		return;
@@ -80,10 +82,14 @@ async function prepareAndBuildWorker(
 		return;
 	}
 
+	// NOTE: Middleware manifest logic will be removed in the new routing system. (see issue #129)
 	let middlewareManifestData: MiddlewareManifestData;
 
 	try {
-		middlewareManifestData = await getParsedMiddlewareManifest(functionsMap);
+		middlewareManifestData = await getParsedMiddlewareManifest(
+			functionsMap,
+			nextJsConfigs
+		);
 	} catch (e: unknown) {
 		if (e instanceof Error) {
 			cliError(e.message, true);
@@ -96,9 +102,16 @@ async function prepareAndBuildWorker(
 		exit(1);
 	}
 
-	await buildWorkerFile(
-		middlewareManifestData,
+	const staticAssets = await getVercelStaticAssets();
+
+	const processedVercelOutput = processVercelOutput(
 		vercelConfig,
+		staticAssets,
+		middlewareManifestData
+	);
+
+	await buildWorkerFile(
+		processedVercelOutput,
 		nextJsConfigs,
 		options.experimentalMinify
 	);

diff --git a/src/buildApplication/buildWorkerFile.ts b/src/buildApplication/buildWorkerFile.ts
@@ -4,12 +4,34 @@ import { build } from 'esbuild';
 import { tmpdir } from 'os';
 import { cliLog } from '../cli';
 import { NextJsConfigs } from './nextJsConfigs';
-import { MiddlewareManifestData } from './middlewareManifest';
 import { generateGlobalJs } from './generateGlobalJs';
+import { ProcessedVercelOutput } from './processVercelOutput';
 
+/**
+ * Construct a record for the build output map.
+ *
+ * @param item The build output item to construct a record for.
+ * @returns Record for the build output map.
+ */
+function constructBuildOutputRecord(item: BuildOutputItem) {
+	return item.type === 'static'
+		? `{ type: ${JSON.stringify(item.type)} }`
+		: item.type === 'override'
+		? `{
+				type: ${JSON.stringify(item.type)},
+				path: ${item.path ? JSON.stringify(item.path) : undefined},
+				contentType: ${item.contentType ? JSON.stringify(item.contentType) : undefined}
+			}`
+		: `{
+				type: ${JSON.stringify(item.type)},
+				entrypoint: AsyncLocalStoragePromise.then(() => import('${item.entrypoint}')),
+				matchers: ${JSON.stringify(item.matchers)}
+			}`;
+}
+
+// NOTE: `nextJsConfigs`, and accompanying logic will be removed in the new routing system. (see issue #129)
 export async function buildWorkerFile(
-	{ hydratedMiddleware, hydratedFunctions }: MiddlewareManifestData,
-	vercelConfig: VercelConfig,
+	{ vercelConfig, vercelOutput }: ProcessedVercelOutput,
 	nextJsConfigs: NextJsConfigs,
 	experimentalMinify: boolean
 ) {
@@ -25,22 +47,8 @@ export async function buildWorkerFile(
 			globalThis.AsyncLocalStorage = AsyncLocalStorage;
 		}).catch(() => undefined);
 
-		export const __FUNCTIONS__ = {${[...hydratedFunctions.entries()]
-			.map(
-				([name, { matchers, filepath }]) =>
-					`"${name}": { matchers: ${JSON.stringify(
-						matchers
-					)}, entrypoint: AsyncLocalStoragePromise.then(() => import('${filepath}'))}`
-			)
-			.join(',')}};
-
-		export const __MIDDLEWARE__ = {${[...hydratedMiddleware.entries()]
-			.map(
-				([name, { matchers, filepath }]) =>
-					`"${name}": { matchers: ${JSON.stringify(
-						matchers
-					)}, entrypoint: AsyncLocalStoragePromise.then(() => import('${filepath}'))}`
-			)
+		export const __BUILD_OUTPUT__ = {${[...vercelOutput.entries()]
+			.map(([name, item]) => `"${name}": ${constructBuildOutputRecord(item)}`)
 			.join(',')}};`
 	);
 

diff --git a/src/buildApplication/generateFunctionsMap.ts b/src/buildApplication/generateFunctionsMap.ts
@@ -4,12 +4,14 @@ import { dirname, join, relative } from 'path';
 import { parse, Node } from 'acorn';
 import { generate } from 'astring';
 import {
+	formatRoutePath,
 	normalizePath,
 	readJsonFile,
+	stripIndexRoute,
 	validateDir,
 	validateFile,
 } from '../utils';
-import { cliError, CliOptions } from '../cli';
+import { cliError, CliOptions, cliWarn } from '../cli';
 import { tmpdir } from 'os';
 
 /**
@@ -27,10 +29,9 @@ import { tmpdir } from 'os';
 export async function generateFunctionsMap(
 	functionsDir: string,
 	experimentalMinify: CliOptions['experimentalMinify']
-): Promise<{
-	functionsMap: Map<string, string>;
-	invalidFunctions: Set<string>;
-}> {
+): Promise<
+	Pick<DirectoryProcessingResults, 'functionsMap' | 'invalidFunctions'>
+> {
 	const processingSetup = {
 		functionsDir,
 		tmpFunctionsDir: join(tmpdir(), Math.random().toString(36).slice(2)),
@@ -43,6 +44,8 @@ export async function generateFunctionsMap(
 		functionsDir
 	);
 
+	await tryToFixInvalidFunctions(processingResults);
+
 	if (experimentalMinify) {
 		await buildWebpackChunkFiles(
 			processingResults.webpackChunks,
@@ -53,6 +56,53 @@ export async function generateFunctionsMap(
 	return processingResults;
 }
 
+/**
+ * Process the invalid functions and check whether and valid function was created in the functions
+ * map to override it.
+ *
+ * The build output sometimes generates invalid functions at the root, while still creating the
+ * valid functions. With the base path and route groups, it might create the valid edge function
+ * inside a folder for the route group, but create an invalid one that maps to the same path
+ * at the root.
+ *
+ * When we process the directory, we might add the valid function to the map before we process the
+ * invalid one, so we need to check if the invalid one was added to the map and remove it from the
+ * set if it was.
+ *
+ * If the invalid function is an RSC function (e.g. `path.rsc`) and doesn't have a valid squashed
+ * version, we check if a squashed non-RSC function exists (e.g. `path`) and use this instead. RSC
+ * functions are the same as non-RSC functions, per the Vercel source code.
+ * https://github.com/vercel/vercel/blob/main/packages/next/src/server-build.ts#L1193
+ *
+ * @param processingResults Object containing the results of processing the current function directory.
+ */
+async function tryToFixInvalidFunctions({
+	functionsMap,
+	invalidFunctions,
+}: DirectoryProcessingResults): Promise<void> {
+	if (invalidFunctions.size === 0) {
+		return;
+	}
+
+	for (const rawPath of invalidFunctions) {
+		const formattedPath = formatRoutePath(rawPath);
+
+		if (
+			functionsMap.has(formattedPath) ||
+			functionsMap.has(stripIndexRoute(formattedPath))
+		) {
+			invalidFunctions.delete(rawPath);
+		} else if (formattedPath.endsWith('.rsc')) {
+			const value = functionsMap.get(formattedPath.replace(/\.rsc$/, ''));
+
+			if (value) {
+				functionsMap.set(formattedPath, value);
+				invalidFunctions.delete(rawPath);
+			}
+		}
+	}
+}
+
 async function processDirectoryRecursively(
 	setup: ProcessingSetup,
 	dir: string
@@ -117,8 +167,26 @@ async function processFuncDirectory(
 		};
 	}
 
+	// There are instances where the build output will generate an uncompiled `middleware.js` file that is used as the entrypoint.
+	// TODO: investigate when and where the file is generated.
+	// This file is not able to be used as it is uncompiled, so we try to instead use the compiled `index.js` if it exists.
+	let isMiddleware = false;
+	if (functionConfig.entrypoint === 'middleware.js') {
+		isMiddleware = true;
+		functionConfig.entrypoint = 'index.js';
+	}
+
 	const functionFile = join(filepath, functionConfig.entrypoint);
 	if (!(await validateFile(functionFile))) {
+		if (isMiddleware) {
+			// We sometimes encounter an uncompiled `middleware.js` with no compiled `index.js` outside of a base path.
+			// Outside the base path, it should not be utilised, so it should be safe to ignore the function.
+			cliWarn(
+				`Detected an invalid middleware function for ${relativePath}. Skipping...`
+			);
+			return {};
+		}
+
 		return {
 			invalidFunctions: new Set([file]),
 		};
@@ -143,12 +211,15 @@ async function processFuncDirectory(
 	await mkdir(dirname(newFilePath), { recursive: true });
 	await writeFile(newFilePath, contents);
 
-	functionsMap.set(
-		normalizePath(
-			relative(setup.functionsDir, filepath).slice(0, -'.func'.length)
-		),
-		normalizePath(newFilePath)
-	);
+	const formattedPathName = formatRoutePath(relativePath);
+	const normalizedFilePath = normalizePath(newFilePath);
+
+	functionsMap.set(formattedPathName, normalizedFilePath);
+
+	if (formattedPathName.endsWith('/index')) {
+		// strip `/index` from the path name as the build output config doesn't rewrite `/index` to `/`
+		functionsMap.set(stripIndexRoute(formattedPathName), normalizedFilePath);
+	}
 
 	return {
 		functionsMap,

diff --git a/src/buildApplication/getVercelConfig.ts b/src/buildApplication/getVercelConfig.ts
@@ -25,3 +25,31 @@ export async function getVercelConfig(): Promise<VercelConfig> {
 
 	return config;
 }
+
+export function processVercelConfig(
+	config: VercelConfig
+): ProcessedVercelConfig {
+	const processedConfig: ProcessedVercelConfig = {
+		...config,
+		routes: {
+			none: [],
+			filesystem: [],
+			miss: [],
+			rewrite: [],
+			resource: [],
+			hit: [],
+			error: [],
+		},
+	};
+
+	let currentPhase: VercelHandleValue | 'none' = 'none';
+	config.routes.forEach(route => {
+		if ('handle' in route) {
+			currentPhase = route.handle;
+		} else {
+			processedConfig.routes[currentPhase].push(route);
+		}
+	});
+
+	return processedConfig;
+}
diff --git a/src/buildApplication/middlewareManifest.ts b/src/buildApplication/middlewareManifest.ts
@@ -4,7 +4,10 @@
  *  should be refactored to use .vercel/output instead as soon as possible
  */
 
-import { readJsonFile } from '../utils';
+// NOTE: This file and the corresponding logic will be removed in the new routing system. (see issue #129)
+
+import { readJsonFile, stripIndexRoute, stripRouteGroups } from '../utils';
+import type { NextJsConfigs } from './nextJsConfigs';
 
 export type EdgeFunctionDefinition = {
 	name: string;
@@ -31,7 +34,8 @@ export type MiddlewareManifestData = Awaited<
  * gets the parsed middleware manifest and validates it against the existing functions map.
  */
 export async function getParsedMiddlewareManifest(
-	functionsMap: Map<string, string>
+	functionsMap: Map<string, string>,
+	{ basePath }: NextJsConfigs
 ) {
 	// Annoying that we don't get this from the `.vercel` directory.
 	// Maybe we eventually just construct something similar from the `.vercel/output/functions` directory with the same magic filename/precendence rules?
@@ -42,12 +46,13 @@ export async function getParsedMiddlewareManifest(
 		throw new Error('Could not read the functions manifest.');
 	}
 
-	return parseMiddlewareManifest(middlewareManifest, functionsMap);
+	return parseMiddlewareManifest(middlewareManifest, functionsMap, basePath);
 }
 
 export function parseMiddlewareManifest(
 	middlewareManifest: MiddlewareManifest,
-	functionsMap: Map<string, string>
+	functionsMap: Map<string, string>,
+	basePath?: string
 ) {
 	if (middlewareManifest.version !== 2) {
 		throw new Error(
@@ -74,9 +79,13 @@ export function parseMiddlewareManifest(
 	const functionsEntries = Object.values(middlewareManifest.functions);
 
 	for (const [name, filepath] of functionsMap) {
+		// the .vc-config name includes the basePath, so we need to strip it for matching in the middleware manifest.
+		const prefixRegex = new RegExp(`^(${basePath})?/`);
+		const fileName = name.replace(prefixRegex, '');
+
 		if (
 			middlewareEntries.length > 0 &&
-			(name === 'middleware' || name === 'src/middleware')
+			(fileName === 'middleware' || fileName === 'src/middleware')
 		) {
 			for (const entry of middlewareEntries) {
 				if (entry?.name === 'middleware' || entry?.name === 'src/middleware') {
@@ -86,8 +95,16 @@ export function parseMiddlewareManifest(
 		}
 
 		for (const entry of functionsEntries) {
-			if (matchFunctionEntry(entry.name, name)) {
+			if (matchFunctionEntry(stripRouteGroups(entry.name), fileName)) {
 				hydratedFunctions.set(name, { matchers: entry.matchers, filepath });
+
+				// NOTE: Temporary to account for `/index` routes.
+				if (stripIndexRoute(name) !== name) {
+					hydratedFunctions.set(stripIndexRoute(name), {
+						matchers: entry.matchers,
+						filepath,
+					});
+				}
 			}
 		}
 	}

diff --git a/src/buildApplication/nextJsConfigs/getBasePath.ts b/src/buildApplication/nextJsConfigs/getBasePath.ts
@@ -1,3 +1,5 @@
+// NOTE: This file and the corresponding logic will be removed in the new routing system. (see issue #129)
+
 import { cliWarn } from '../../cli';
 import { readJsonFile } from '../../utils';
 

diff --git a/src/buildApplication/nextJsConfigs/index.ts b/src/buildApplication/nextJsConfigs/index.ts
@@ -1,3 +1,5 @@
+// NOTE: This file and the corresponding logic will be removed in the new routing system. (see issue #129)
+
 import { getBasePath } from './getBasePath';
 
 export type NextJsConfigs = {