implement route scoping solution

.changeset/thirty-birds-build.md

@@ -0,0 +1,30 @@
+---
+'@cloudflare/next-on-pages': patch
+---
+
+fix: implement route specific global scoping strategy
+
+currently routes all share the same global scope, this can be problematic and cause
+race conditions and failures
+
+One example of this is the following code that is present in route function files:
+
+```ts
+self.webpackChunk_N_E = ...
+```
+
+and
+
+```ts
+self.webpackChunk_N_E.push(...)
+```
+
+this indicates that an in-memory global collection of the webpack chunks is shared by all routes,
+this combined with the fact that chunks can have their own module state this can easily cause routes to conflict with each other at runtime.
+
+So, in order to solve the above issue, all route functions are wrapped in a function which accepts as parameters, thus overrides, the `self`, `globalThis` and `global` symbols. The symbols
+will be resolved with proxies that redirect setters to route-scoped in-memory maps and
+getters to the above mentioned map's values and fallback to the original symbol values otherwise
+(i.e. `globalThis` will be overridden by a proxy that, when setting values, sets them in a separate
+location and, when getting values, gets them from said location if present there or from the real
+`globalThis` otherwise)

packages/next-on-pages/src/buildApplication/processVercelFunctions/dedupeEdgeFunctions.ts

@@ -101,13 +101,20 @@ async function processFunctionIdentifiers(
 
 			if (importPath) {
 				// Dedupe and update collected imports.
-				const { updatedContents } = await processImportIdentifier(
-					{ type, identifier, start, end, importPath, info: identifierInfo },
-					{ fileContents, entrypoint, newFnLocation, fnConfig: fnInfo.config },
-					opts,
-				);
+				const { updatedContents, newImportToPrepend } =
+					await processImportIdentifier(
+						{ type, identifier, start, end, importPath, info: identifierInfo },
+						{
+							fileContents,
+							entrypoint,
+							newFnLocation,
+							fnConfig: fnInfo.config,
+						},
+						opts,
+					);
 
 				fileContents = updatedContents;
+				importsToPrepend.push(newImportToPrepend);
 			} else if (identifierInfo.consumers.length > 1) {
 				// Only dedupe code blocks if there are multiple consumers.
 				const { updatedContents, newFilePath, newImport, wasmImports } =
@@ -143,9 +150,10 @@ async function processFunctionIdentifiers(
 
 		// Build the identifier files before building the function's file.
 		await Promise.all(
-			[...identifierPathsToBuild].map(async path =>
-				buildFile(await readFile(path, 'utf8'), path),
-			),
+			[...identifierPathsToBuild].map(async path => {
+				const fileContents = await functionifyFileContent(path);
+				return buildFile(fileContents, path);
+			}),
 		);
 
 		// If wasm identifier is used in code block, prepend the import to the code block's file.
@@ -168,6 +176,27 @@ async function processFunctionIdentifiers(
 	await Promise.all(functionBuildPromises);
 }
 
+/**
+ * Given a standard ESM file (without imports) it converts it to a function call that returns
+ * an object with the various exports set as its fields
+ *
+ * The function allows us to override global symbols such as `self`, `globalThis` and `global`
+ * (which are used as the function's parameter names)
+ *
+ * @param path the path of the ESM file
+ * @returns the converted file content
+ */
+async function functionifyFileContent(path: string) {
+	const originalFileContents = await readFile(path, 'utf8');
+	return `
+		${
+			/* Note: we need to make sure that the named exports object is defined since that is used inside the file */ ''
+		}
+		const ${namedExportsObjectName} = {};
+		export const ${getNamedExportsFunctionName} = ((self, globalThis, global) => { ${originalFileContents} return ${namedExportsObjectName}; });
+	`;
+}
+
 /**
  * Builds a new file for an Edge function.
  *
@@ -184,7 +213,7 @@ async function buildFunctionFile(
 	{ importsToPrepend }: { importsToPrepend: NewImportInfo[] },
 	{ workerJsDir, nopDistDir }: ProcessVercelFunctionsOpts,
 ): Promise<{ buildPromise: Promise<void> }> {
-	let functionImports = '';
+	const functionImports: string[] = [];
 
 	// Group the identifier imports by the keys for each path.
 	const groupedImports = importsToPrepend.reduce((acc, { key, path }) => {
@@ -193,21 +222,43 @@ async function buildFunctionFile(
 		return acc;
 	}, new Map<string, string>());
 
-	groupedImports.forEach((keys, path) => {
-		const relativeImportPath = getRelativePathToAncestor({
-			from: newFnLocation,
-			relativeTo: nopDistDir,
-		});
+	let chunkMapIdx = 0;
+	const chunksExportsMap = new Map<string, Set<string>>();
+
+	const relativeImportPath = getRelativePathToAncestor({
+		from: newFnLocation,
+		relativeTo: nopDistDir,
+	});
+
+	groupedImports.forEach((exports, path) => {
 		const importPath = normalizePath(
 			join(relativeImportPath, addLeadingSlash(path)),
 		);
 
-		functionImports += `import { ${keys} } from '${importPath}';\n`;
+		if (path.endsWith('.wasm')) {
+			// if we're dealing with a wasm file there is a single default export to deal with
+			const defaultExport = exports;
+			// we don't need/want to apply any code transformation for wasm imports
+			functionImports.push(`import ${defaultExport} from '${path}';`);
+			return;
+		}
+
+		const getNamedExportsFunctionWithId = `${getNamedExportsFunctionName}_${chunkMapIdx++}`;
+		const exportKeys = exports.split(',');
+		chunksExportsMap.set(getNamedExportsFunctionWithId, new Set(exportKeys));
+		functionImports.push(
+			`import { ${getNamedExportsFunctionName} as ${getNamedExportsFunctionWithId} } from '${importPath}';`,
+		);
 	});
 
 	fnInfo.outputPath = relative(workerJsDir, newFnPath);
 
-	const finalFileContents = `${functionImports}${fileContents}`;
+	const finalFileContents = iifefyFunctionFile(
+		fileContents,
+		functionImports,
+		fnInfo,
+		chunksExportsMap,
+	);
 	const buildPromise = buildFile(finalFileContents, newFnPath, {
 		relativeTo: nopDistDir,
 	}).then(async () => {
@@ -225,6 +276,60 @@ type BuildFunctionFileOpts = {
 	newFnPath: string;
 };
 
+/**
+ * Given the content of a function file it converts/wraps it into an iife that overrides the function's contents with an iffe call that
+ * overrides global symbols with route-specific proxies (for more details see: templates/_worker.js/routesIsolation.ts)
+ *
+ * @param fileContents the function file's contents
+ * @param functionImports the imports that need to be added to the file
+ * @param functionInfo the function's information
+ * @param chunksExportsMap a map containing getters and chunks identifiers being used by the function
+ * @returns the updated/iifefied file content
+ */
+function iifefyFunctionFile(
+	fileContents: string,
+	functionImports: string[],
+	functionInfo: FunctionInfo,
+	chunksExportsMap: Map<string, Set<string>>,
+): string {
+	const wrappedContent = `
+		export default ((self, globalThis, global) => {
+			${fileContents
+				// it looks like there can be direct references to _ENTRIES (i.e. `_ENTRIES` instead of `globalThis._ENTRIES` etc...)
+				// we have to update all such references otherwise our proxying won't take effect on those
+				// (note: older versions of the Vercel CLI (v31 and older) used to declare _ENTRIES as "let _ENTRIES = {};", so we do
+				// need to make sure that we don't add `globalThis.` in these cases (if we were to drop support for those older versions
+				// the below line to: ".replace(/([^.])_ENTRIES/g, '$1globalThis._ENTRIES')")
+				.replace(/(?<!(let)\s*)([^.]|^)_ENTRIES/g, '$2globalThis._ENTRIES')
+				// the default export needs to become the return value of the iife, which is then re-exported as default
+				.replace(/export\s+default\s+/g, 'return ')}
+		})(proxy, proxy, proxy);
+	`;
+
+	const proxyCall = `const proxy = globalThis.__nextOnPagesRoutesIsolation.getProxyFor('${
+		functionInfo.route?.path ?? ''
+	}');`;
+
+	const chunksExtraction = [...chunksExportsMap.entries()].flatMap(
+		([getNamedExportsFunctionWithId, keys]) => {
+			const namedExportsObjectWithId = `__exportsOf${getNamedExportsFunctionWithId}`;
+			return [
+				`const ${namedExportsObjectWithId} = ${getNamedExportsFunctionWithId}(proxy, proxy, proxy);`,
+				...[...keys.keys()].map(
+					key => `const ${key} = ${namedExportsObjectWithId}["${key}"];`,
+				),
+			];
+		},
+	);
+
+	return [
+		...functionImports,
+		proxyCall,
+		...chunksExtraction,
+		wrappedContent,
+	].join('\n');
+}
+
 /**
  * Prepends Wasm imports to a code block's built file.
  *
@@ -246,19 +351,22 @@ async function prependWasmImportsToCodeBlocks(
 					relativeTo: nopDistDir,
 				});
 
-				let functionImports = '';
+				const functionImports: string[] = [];
 
 				for (const identifier of wasmImports) {
 					// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
 					const { newDest } = identifierMaps.wasm.get(identifier)!;
 					const wasmImportPath = normalizePath(
 						join(relativeImportPath, newDest as string),
 					);
-					functionImports += `import ${identifier} from "${wasmImportPath}";\n`;
+					functionImports.push(`import ${identifier} from "${wasmImportPath}"`);
 				}
 
 				const oldContents = await readFile(filePath);
-				await writeFile(filePath, `${functionImports}${oldContents}`);
+				await writeFile(
+					filePath,
+					`${functionImports.join(';')};${oldContents}`,
+				);
 			},
 		),
 	);
@@ -268,12 +376,13 @@ async function prependWasmImportsToCodeBlocks(
  * Processes an import path identifier.
  *
  * - Moves the imported file to the new location if it doesn't exist.
- * - Updates the file contents to import from the new location.
+ * - Updates the file contents to remove the import.
+ * - Returns the information for the new import.
  *
  * @param ident The import path identifier to process.
  * @param processOpts Contents of the function's file, the function's entrypoint, and the new path.
  * @param opts Options for processing the function.
- * @returns The updated file contents.
+ * @returns The updated file contents alongside the new import information.
  */
 async function processImportIdentifier(
 	ident: RawIdentifierWithImport<IdentifierType> & { info: IdentifierInfo },
@@ -284,7 +393,7 @@ async function processImportIdentifier(
 		fnConfig,
 	}: ProcessImportIdentifierOpts,
 	{ nopDistDir, workerJsDir }: ProcessVercelFunctionsOpts,
-): Promise<{ updatedContents: string }> {
+): Promise<{ updatedContents: string; newImportToPrepend: NewImportInfo }> {
 	const { type, identifier, start, end, importPath, info } = ident;
 	let updatedContents = fileContents;
 
@@ -320,14 +429,20 @@ async function processImportIdentifier(
 	});
 	const newImportPath = normalizePath(join(relativeImportPath, info.newDest));
 
-	const newVal = `import ${identifier} from "${newImportPath}";`;
+	// let's remove the original import since it will be re-added later when appropriate
 	updatedContents = replaceLastSubstringInstance(
 		updatedContents,
 		codeBlock,
-		newVal,
+		'',
 	);
 
-	return { updatedContents };
+	return {
+		updatedContents,
+		newImportToPrepend: {
+			key: identifier,
+			path: newImportPath,
+		},
+	};
 }
 
 type ProcessImportIdentifierOpts = {
@@ -383,7 +498,7 @@ async function processCodeBlockIdentifier(
 			.forEach(key => wasmImports.push(key));
 
 		const buffer = Buffer.from(
-			`export const ${identifierKey} = ${codeBlock}\n`,
+			`${namedExportsObjectName}["${identifierKey}"] = ${codeBlock}\n`,
 			'utf8',
 		);
 
@@ -554,3 +669,43 @@ async function processBundledAssets(
 		}
 	}
 }
+
+/**
+ * When performing the various code tweaking we never introduce standard named ESM exports, since those would
+ * be invalid anyways since each js file content gets wrapped into a function anyways.
+ *
+ * Note: route function files don't have named exports to this is only needed for the other files such
+ *       as the manifest ones and the webpack chunks ones
+ *
+ * Instead of standard named exports we simply set named exports onto an object which gets then returned by the
+ * file wrapper function.
+ *
+ *
+ * Example:
+ *   when introducing a new export, instead of introducing:
+ *   ```
+ *   export const a = ...
+ *   ```
+ *   we introduce something like:
+ *   ```
+ *   NAMED_EXPORTS_OBJECT["a"] = ...
+ *   ```
+ *   and make sure that such <NAMED_EXPORTS_OBJECT> object is always declared and returned by the function wrapping the file's contents.
+ *
+ * This is the name of the object used for such exports.
+ */
+const namedExportsObjectName = '__namedExportsObject';
+
+/**
+ * The strategy of exporting exports via an returned object relies on the fact that there is a function that returns such object.
+ *
+ * Example:
+ * ```
+ *  import { __getNamedExports } from '...';
+ *  const obj = __getNamedExports(...);
+ *  // obj contains the various exports as standard object properties
+ * ```
+ *
+ * This is the name of such function.
+ */
+const getNamedExportsFunctionName = '__getNamedExports';

packages/next-on-pages/templates/_worker.js/index.ts

@@ -1,5 +1,6 @@
 import { SUSPENSE_CACHE_URL } from '../cache';
 import { handleRequest } from './handleRequest';
+import { setupRoutesIsolation } from './routesIsolation';
 import {
 	adjustRequestForVercel,
 	handleImageResizingRequest,
@@ -22,6 +23,7 @@ declare const __ALSes_PROMISE__: Promise<null | {
 
 export default {
 	async fetch(request, env, ctx) {
+		setupRoutesIsolation();
 		patchFetch();
 
 		const asyncLocalStorages = await __ALSes_PROMISE__;