add $ to all route srcs (#473)

* add $ to all route srcs

* improve code

* fix e2es in regard of error page

packages/next-on-pages/src/buildApplication/getVercelConfig.ts

@@ -70,7 +70,7 @@ export function processVercelConfig(
 	config: VercelConfig,
 ): ProcessedVercelConfig {
 	const processedConfig: ProcessedVercelConfig = {
-		...config,
+		...JSON.parse(JSON.stringify(config)),
 		routes: {
 			none: [],
 			filesystem: [],
@@ -84,23 +84,48 @@ export function processVercelConfig(
 
 	let currentPhase: VercelHandleValue | 'none' = 'none';
 	config.routes?.forEach(route => {
-		// Vercel generates `^/` as the regex instead of `^/$` for their root prefer route.
-		// This makes the route match everything and not only the actual root requests, so
-		// here we need to fix such erroneous regex.
-		if (route.src === '^/' && route.dest === '/index.prefetch.rsc') {
-			route.src = '^/$';
-		}
-
 		if (isVercelHandler(route)) {
 			currentPhase = route.handle;
 		} else {
+			normalizeRouteSrc(route);
 			processedConfig.routes[currentPhase].push(route);
 		}
 	});
 
 	return processedConfig;
 }
 
+/**
+ * Given a source route it normalizes its src value if needed.
+ *
+ * (In this context normalization means tweaking the src value so that it follows
+ * a format which Vercel expects).
+ *
+ * Note: this function applies the change side-effectfully to the route object.
+ *
+ * @param route Route which src we want to potentially normalize
+ */
+function normalizeRouteSrc(route: VercelSource): void {
+	if (!route.src) return;
+
+	// we rely on locale root routes pointing to '/' to perform runtime checks
+	// so we cannot normalize such src values as that would break things later on
+	// see: https://github.com/cloudflare/next-on-pages/blob/654545/packages/next-on-pages/templates/_worker.js/routes-matcher.ts#L353-L358
+	if (route.locale && route.src === '/') return;
+
+	// Route src should always start with a '^'
+	// see: https://github.com/vercel/vercel/blob/ea5bc88/packages/routing-utils/src/index.ts#L77
+	if (!route.src.startsWith('^')) {
+		route.src = `^${route.src}`;
+	}
+
+	// Route src should always end with a '$'
+	// see: https://github.com/vercel/vercel/blob/ea5bc88/packages/routing-utils/src/index.ts#L82
+	if (!route.src.endsWith('$')) {
+		route.src = `${route.src}$`;
+	}
+}
+
 function isVercelHandler(route: VercelRoute): route is VercelHandler {
 	return 'handle' in route;
 }

packages/next-on-pages/templates/_worker.js/routes-matcher.ts

@@ -393,10 +393,6 @@ export class RoutesMatcher {
 	 * Modifies the source route's `src` regex to be friendly with previously found locale's in the
 	 * `miss` phase.
 	 *
-	 * Sometimes, there is a source route with `src: '/{locale}'`, which rewrites all paths containing
-	 * the locale to `/`. This is problematic for matching, and should only do this if the path is
-	 * exactly the locale, i.e. `^/{locale}$`.
-	 *
 	 * There is a source route generated for rewriting `/{locale}/*` to `/*` when no file was found
 	 * for the path. This causes issues when using an SSR function for the index page as the request
 	 * to `/{locale}` will not be caught by the regex. Therefore, the regex needs to be updated to
@@ -414,14 +410,11 @@ export class RoutesMatcher {
 			return route;
 		}
 
-		const isLocaleIndex =
-			/^\//.test(route.src) && route.src.slice(1) in this.locales;
-		if (isLocaleIndex) {
-			return { ...route, src: `^${route.src}$` };
-		}
-
 		if (isLocaleTrailingSlashRegex(route.src, this.locales)) {
-			return { ...route, src: route.src.replace(/\/\(\.\*\)$/, '(?:/(.*))?$') };
+			return {
+				...route,
+				src: route.src.replace(/\/\(\.\*\)\$$/, '(?:/(.*))?$'),
+			};
 		}
 
 		return route;

packages/next-on-pages/templates/_worker.js/utils/routing.ts

@@ -138,7 +138,7 @@ export async function runOrFetchBuildOutputItem(
  * Checks if a source route's matcher uses the regex format for locales with a trailing slash, where
  * the locales specified are known.
  *
- * Determines whether a matcher is in the format of `^//?(?:en|fr|nl)/(.*)`.
+ * Determines whether a matcher is in the format of `^//?(?:en|fr|nl)/(.*)$`.
  *
  * @param src Source route `src` regex value.
  * @param locales Known available locales.
@@ -149,7 +149,7 @@ export function isLocaleTrailingSlashRegex(
 	locales: Record<string, string>,
 ) {
 	const prefix = '^//?(?:';
-	const suffix = ')/(.*)';
+	const suffix = ')/(.*)$';
 
 	if (!src.startsWith(prefix) || !src.endsWith(suffix)) {
 		return false;

packages/next-on-pages/tests/src/buildApplication/getVercelConfig.test.ts

@@ -20,9 +20,9 @@ describe('processVercelConfig', () => {
 		expect(processVercelConfig(inputtedConfig)).toEqual({
 			version: 3,
 			routes: {
-				none: [{ src: '/test-1', dest: '/test-2' }],
-				filesystem: [{ src: '/test-3', dest: '/test-4' }],
-				miss: [{ src: '/test-2', dest: '/test-6' }],
+				none: [{ src: '^/test-1$', dest: '/test-2' }],
+				filesystem: [{ src: '^/test-3$', dest: '/test-4' }],
+				miss: [{ src: '^/test-2$', dest: '/test-6' }],
 				rewrite: [],
 				resource: [],
 				hit: [],

packages/next-on-pages/tests/src/buildApplication/processVercelOutput.test.ts

@@ -55,11 +55,11 @@ describe('processVercelOutput', () => {
 				version: 3,
 				routes: {
 					none: [
-						{ src: '/test-1', dest: '/test-2' },
-						{ src: '/use-middleware', middlewarePath: 'middleware' },
+						{ src: '^/test-1$', dest: '/test-2' },
+						{ src: '^/use-middleware$', middlewarePath: 'middleware' },
 					],
-					filesystem: [{ src: '/test-3', dest: '/test-4' }],
-					miss: [{ src: '/test-2', dest: '/test-6' }],
+					filesystem: [{ src: '^/test-3$', dest: '/test-4' }],
+					miss: [{ src: '^/test-2$', dest: '/test-6' }],
 					rewrite: [],
 					resource: [],
 					hit: [],

pages-e2e/features/appRouting/500.test.ts

@@ -2,18 +2,14 @@ import { describe, test } from 'vitest';
 import { getAssertVisible } from '@features-utils/getAssertVisible';
 
 describe('default error page', () => {
-	test(`visiting a page that throws results in the default 500 internal server error page`, async () => {
+	test(`visiting a page that throws results in the default server error page`, async () => {
 		const page = await BROWSER.newPage();
 		const assertVisible = getAssertVisible(page);
 
 		await page.goto(`${DEPLOYMENT_URL}/500-error`);
 
-		await assertVisible('h1', {
-			hasText: '500',
-		});
-
 		await assertVisible('h2', {
-			hasText: 'Internal Server Error.',
+			hasText: 'Application error',
 		});
 	});
 });