Make sure that trailingSlashes don't interfere with the path existenc…

…e check (#594)

* update pathExistsInOutput check in regards to trailing slashes

* add e2e tests for issue-593

.changeset/thirty-turtles-speak.md

@@ -0,0 +1,9 @@
+---
+'@cloudflare/next-on-pages': patch
+---
+
+Make route matching check handle better trailing slashes
+
+Currently having `trailingSlash` set to `true` in the `next.config.js` file
+results in some routes not being correctly handled, this fix addresses such
+issue

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

@@ -608,10 +608,9 @@ export class RoutesMatcher {
 
 		let pathExistsInOutput = this.path in this.output;
 
-		// If a path with a trailing slash entered the `rewrite` phase and didn't find a match, it might
-		// be due to the `trailingSlash` setting in `next.config.js`. Therefore, we should remove the
-		// trailing slash and check again before entering the next phase.
-		if (phase === 'rewrite' && !pathExistsInOutput && this.path.endsWith('/')) {
+		// paths could incorrectly not be detected as existing in the output due to the `trailingSlash` setting
+		// in `next.config.js`, so let's check for that here and update the path in such case
+		if (!pathExistsInOutput && this.path.endsWith('/')) {
 			const newPath = this.path.replace(/\/$/, '');
 			pathExistsInOutput = newPath in this.output;
 			if (pathExistsInOutput) {

pages-e2e/features/issue593/issue.test.ts

@@ -0,0 +1,29 @@
+import { getAssertVisible } from '@features-utils/getAssertVisible';
+import { describe, test } from 'vitest';
+
+describe('issue-593', () => {
+	test('navigating to /api/hello should return a Hello world response', async ({
+		expect,
+	}) => {
+		const response = await fetch(`${DEPLOYMENT_URL}/api/hello`);
+		expect(await response.json()).toEqual({
+			text: 'Hello world!',
+		});
+	});
+
+	test('/some-random-route should display the catch-all page', async () => {
+		const page = await BROWSER.newPage();
+		const assertVisible = getAssertVisible(page);
+		const pageUrl = `${DEPLOYMENT_URL}/some-random-route`;
+		await page.goto(pageUrl);
+		await assertVisible('h1', { hasText: 'catch-all route' });
+	});
+
+	test('/ should display the home page', async () => {
+		const page = await BROWSER.newPage();
+		const assertVisible = getAssertVisible(page);
+		const pageUrl = `${DEPLOYMENT_URL}/`;
+		await page.goto(pageUrl);
+		await assertVisible('h1', { hasText: 'home page' });
+	});
+});

pages-e2e/features/issue593/main.feature

@@ -0,0 +1,3 @@
+{
+	"setup": "node --loader tsm setup.ts"
+}

pages-e2e/features/issue593/setup.ts

@@ -0,0 +1 @@
+// no setup required

pages-e2e/fixtures/issue593App/.gitignore

@@ -0,0 +1,36 @@
+# See https://help.github.com/articles/ignoring-files/ for more about ignoring files.
+
+# dependencies
+/node_modules
+/.pnp
+.pnp.js
+.yarn/install-state.gz
+
+# testing
+/coverage
+
+# next.js
+/.next/
+/out/
+
+# production
+/build
+
+# misc
+.DS_Store
+*.pem
+
+# debug
+npm-debug.log*
+yarn-debug.log*
+yarn-error.log*
+
+# local env files
+.env*.local
+
+# vercel
+.vercel
+
+# typescript
+*.tsbuildinfo
+next-env.d.ts

pages-e2e/fixtures/issue593App/README.md

@@ -0,0 +1,40 @@
+This is a [Next.js](https://nextjs.org/) project bootstrapped with [`create-next-app`](https://github.com/vercel/next.js/tree/canary/packages/create-next-app).
+
+## Getting Started
+
+First, run the development server:
+
+```bash
+npm run dev
+# or
+yarn dev
+# or
+pnpm dev
+# or
+bun dev
+```
+
+Open [http://localhost:3000](http://localhost:3000) with your browser to see the result.
+
+You can start editing the page by modifying `pages/index.tsx`. The page auto-updates as you edit the file.
+
+[API routes](https://nextjs.org/docs/api-routes/introduction) can be accessed on [http://localhost:3000/api/hello](http://localhost:3000/api/hello). This endpoint can be edited in `pages/api/hello.ts`.
+
+The `pages/api` directory is mapped to `/api/*`. Files in this directory are treated as [API routes](https://nextjs.org/docs/api-routes/introduction) instead of React pages.
+
+This project uses [`next/font`](https://nextjs.org/docs/basic-features/font-optimization) to automatically optimize and load Inter, a custom Google Font.
+
+## Learn More
+
+To learn more about Next.js, take a look at the following resources:
+
+- [Next.js Documentation](https://nextjs.org/docs) - learn about Next.js features and API.
+- [Learn Next.js](https://nextjs.org/learn) - an interactive Next.js tutorial.
+
+You can check out [the Next.js GitHub repository](https://github.com/vercel/next.js/) - your feedback and contributions are welcome!
+
+## Deploy on Vercel
+
+The easiest way to deploy your Next.js app is to use the [Vercel Platform](https://vercel.com/new?utm_medium=default-template&filter=next.js&utm_source=create-next-app&utm_campaign=create-next-app-readme) from the creators of Next.js.
+
+Check out our [Next.js deployment documentation](https://nextjs.org/docs/deployment) for more details.

pages-e2e/fixtures/issue593App/main.fixture

@@ -0,0 +1,12 @@
+{
+	"features": [
+		"issue593"
+	],
+	"buildConfig": {
+		"buildCommand": "npx --force ../../../packages/next-on-pages",
+		"buildOutputDirectory": ".vercel/output/static"
+	},
+	"deploymentConfig": {
+		"compatibilityFlags": ["nodejs_compat"]
+	}
+}

pages-e2e/fixtures/issue593App/next.config.js

@@ -0,0 +1,7 @@
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+	trailingSlash: true,
+	reactStrictMode: true,
+};
+
+module.exports = nextConfig;