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/);
+ });
 });