fix(rewrite): match index with params (#11065)

* fix(rewrite): match index with params * remove import
withastro · May 16, 2024 · 1f988ed · 1f988ed
1 parent 87f36d4
commit 1f988ed
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 17 deletions.
diff --git a/.changeset/brave-cycles-suffer.md b/.changeset/brave-cycles-suffer.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes a bug in the Astro rewrite logic, where rewriting the index with parameters - `next("/?foo=bar")` - didn't work as expected.
diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts
@@ -71,7 +71,10 @@ export class AppPipeline extends Pipeline {
 		return module.page();
 	}
 
-	async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> {
+	async tryRewrite(
+		payload: RewritePayload,
+		request: Request
+	): Promise<[RouteData, ComponentInstance]> {
 		let foundRoute;
 
 		for (const route of this.#manifestData!.routes) {
@@ -86,9 +89,12 @@ export class AppPipeline extends Pipeline {
 					foundRoute = route;
 					break;
 				}
-			} else if (route.pattern.test(decodeURI(payload))) {
-				foundRoute = route;
-				break;
+			} else {
+				const newUrl = new URL(payload, new URL(request.url).origin);
+				if (route.pattern.test(decodeURI(newUrl.pathname))) {
+					foundRoute = route;
+					break;
+				}
 			}
 		}
 

diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
@@ -71,7 +71,10 @@ export abstract class Pipeline {
 	 *
 	 * @param {RewritePayload} rewritePayload
 	 */
-	abstract tryRewrite(rewritePayload: RewritePayload): Promise<[RouteData, ComponentInstance]>;
+	abstract tryRewrite(
+		rewritePayload: RewritePayload,
+		request: Request
+	): Promise<[RouteData, ComponentInstance]>;
 
 	/**
 	 * Tells the pipeline how to retrieve a component give a `RouteData`

diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts
@@ -276,7 +276,10 @@ export class BuildPipeline extends Pipeline {
 		}
 	}
 
-	async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> {
+	async tryRewrite(
+		payload: RewritePayload,
+		request: Request
+	): Promise<[RouteData, ComponentInstance]> {
 		let foundRoute: RouteData | undefined;
 		// options.manifest is the actual type that contains the information
 		for (const route of this.options.manifest.routes) {
@@ -291,9 +294,12 @@ export class BuildPipeline extends Pipeline {
 					foundRoute = route;
 					break;
 				}
-			} else if (route.pattern.test(decodeURI(payload))) {
-				foundRoute = route;
-				break;
+			} else {
+				const newUrl = new URL(payload, new URL(request.url).origin);
+				if (route.pattern.test(decodeURI(newUrl.pathname))) {
+					foundRoute = route;
+					break;
+				}
 			}
 		}
 		if (foundRoute) {

diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
@@ -4,7 +4,6 @@ import type {
 	AstroGlobalPartial,
 	ComponentInstance,
 	MiddlewareHandler,
-	MiddlewareNext,
 	RewritePayload,
 	RouteData,
 	SSRResult,
@@ -118,7 +117,7 @@ export class RenderContext {
 			if (payload) {
 				if (this.pipeline.manifest.rewritingEnabled) {
 					try {
-						const [routeData, component] = await pipeline.tryRewrite(payload);
+						const [routeData, component] = await pipeline.tryRewrite(payload, this.request);
 						this.routeData = routeData;
 						componentInstance = component;
 					} catch (e) {
@@ -212,7 +211,7 @@ export class RenderContext {
 		const rewrite = async (reroutePayload: RewritePayload) => {
 			pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload);
 			try {
-				const [routeData, component] = await pipeline.tryRewrite(reroutePayload);
+				const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request);
 				this.routeData = routeData;
 				if (reroutePayload instanceof Request) {
 					this.request = reroutePayload;
@@ -398,7 +397,7 @@ export class RenderContext {
 		const rewrite = async (reroutePayload: RewritePayload) => {
 			try {
 				pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload);
-				const [routeData, component] = await pipeline.tryRewrite(reroutePayload);
+				const [routeData, component] = await pipeline.tryRewrite(reroutePayload, this.request);
 				this.routeData = routeData;
 				if (reroutePayload instanceof Request) {
 					this.request = reroutePayload;

diff --git a/packages/astro/src/vite-plugin-astro-server/pipeline.ts b/packages/astro/src/vite-plugin-astro-server/pipeline.ts
@@ -191,7 +191,10 @@ export class DevPipeline extends Pipeline {
 		}
 	}
 
-	async tryRewrite(payload: RewritePayload): Promise<[RouteData, ComponentInstance]> {
+	async tryRewrite(
+		payload: RewritePayload,
+		request: Request
+	): Promise<[RouteData, ComponentInstance]> {
 		let foundRoute;
 		if (!this.manifestData) {
 			throw new Error('Missing manifest data. This is an internal error, please file an issue.');
@@ -209,9 +212,12 @@ export class DevPipeline extends Pipeline {
 					foundRoute = route;
 					break;
 				}
-			} else if (route.pattern.test(decodeURI(payload))) {
-				foundRoute = route;
-				break;
+			} else {
+				const newUrl = new URL(payload, new URL(request.url).origin);
+				if (route.pattern.test(decodeURI(newUrl.pathname))) {
+					foundRoute = route;
+					break;
+				}
 			}
 		}
 

diff --git a/packages/astro/test/fixtures/reroute/src/middleware.js b/packages/astro/test/fixtures/reroute/src/middleware.js
@@ -18,6 +18,10 @@ export const second = async (context, next) => {
 		if (context.url.pathname.includes('/auth/base')) {
 			return await next('/');
 		}
+
+		if (context.url.pathname.includes('/auth/params')) {
+			return  next('/?foo=bar');
+		}
 	}
 	return next();
 };

diff --git a/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro
@@ -0,0 +1,10 @@
+---
+---
+<html>
+<head>
+	<title>Index with params</title>
+</head>
+<body>
+<h1>Index with params</h1>
+</body>
+</html>
diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js
@@ -220,4 +220,11 @@ describe('Middleware', () => {
 		assert.equal($('h1').text(), 'Index');
 		assert.equal($('p').text(), '');
 	});
+
+	it('should render the index when rewriting with params', async () => {
+		const html = await fixture.fetch('/auth/params').then((res) => res.text());
+		const $ = cheerioLoad(html);
+
+		assert.match($('h1').text(), /Index/);
+	});
 });