From 0b79adcca03c375a5818f707da8e3b17ec9ececf Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 16 May 2024 14:13:37 +0100 Subject: [PATCH] fix(rewrite): match index with params --- .changeset/brave-cycles-suffer.md | 5 +++++ packages/astro/src/core/app/pipeline.ts | 14 ++++++++++---- packages/astro/src/core/base-pipeline.ts | 5 ++++- packages/astro/src/core/build/pipeline.ts | 14 ++++++++++---- packages/astro/src/core/render-context.ts | 7 +++---- .../src/vite-plugin-astro-server/pipeline.ts | 15 +++++++++++---- .../astro/test/fixtures/reroute/src/middleware.js | 4 ++++ .../fixtures/reroute/src/pages/auth/params.astro | 10 ++++++++++ packages/astro/test/rewrite.test.js | 7 +++++++ 9 files changed, 64 insertions(+), 17 deletions(-) create mode 100644 .changeset/brave-cycles-suffer.md create mode 100644 packages/astro/test/fixtures/reroute/src/pages/auth/params.astro diff --git a/.changeset/brave-cycles-suffer.md b/.changeset/brave-cycles-suffer.md new file mode 100644 index 000000000000..1a8de425661d --- /dev/null +++ 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 index f62dc84ed983..0f59a8b047d2 100644 --- 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 index 11cff7c809f5..b4ba9e960407 100644 --- 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 index 7661b6fc4d66..664d30c9ad7a 100644 --- 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 index 5f35c5f11309..4a5cad1da54c 100644 --- 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 index 797c5d29abdd..c8d2b8279ec6 100644 --- a/packages/astro/src/vite-plugin-astro-server/pipeline.ts +++ b/packages/astro/src/vite-plugin-astro-server/pipeline.ts @@ -26,6 +26,7 @@ import { getComponentMetadata } from './metadata.js'; import { createResolve } from './resolve.js'; import { default404Page } from './response.js'; import { getScriptsForURL } from './scripts.js'; +import { matchRoute } from './route.js'; export class DevPipeline extends Pipeline { // renderers are loaded on every request, @@ -191,7 +192,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 +213,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 index 4d7c2a7956c8..09f3ef73c4ad 100644 --- 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 new file mode 100644 index 000000000000..3f4dfad8754b --- /dev/null +++ b/packages/astro/test/fixtures/reroute/src/pages/auth/params.astro @@ -0,0 +1,10 @@ +--- +--- + + + Index with params + + +

Index with params

+ + diff --git a/packages/astro/test/rewrite.test.js b/packages/astro/test/rewrite.test.js index 1c76ce10af74..ef6a90e5dcc0 100644 --- 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/); + }); });