From b29fd011f1a0a5d846b0c896b8b52079810edf11 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 26 Jun 2024 14:07:22 +0100 Subject: [PATCH 1/2] fix: check experimental flag when using the rewrite function --- .changeset/curvy-elephants-fry.md | 5 ++ packages/astro/src/core/render-context.ts | 88 ++++++++++++++--------- 2 files changed, 61 insertions(+), 32 deletions(-) create mode 100644 .changeset/curvy-elephants-fry.md diff --git a/.changeset/curvy-elephants-fry.md b/.changeset/curvy-elephants-fry.md new file mode 100644 index 000000000000..41bbd9e4c3eb --- /dev/null +++ b/.changeset/curvy-elephants-fry.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes an issue where Astro didn't throw an error when `Astro.rewrite` was used without providing the experimental flag diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index e6ac423648ad..fee594d408f5 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -147,7 +147,7 @@ export class RenderContext { componentInstance = component; this.isRewriting = true; } else { - this.pipeline.logger.warn( + this.pipeline.logger.error( 'router', 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' ); @@ -239,23 +239,35 @@ export class RenderContext { const rewrite = async (reroutePayload: RewritePayload) => { pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload); - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; + if (this.pipeline.manifest.rewritingEnabled) { + const [routeData, component, newURL] = await pipeline.tryRewrite( + reroutePayload, + this.request, + this.originalRoute + ); + this.routeData = routeData; + if (reroutePayload instanceof Request) { + this.request = reroutePayload; + } else { + this.request = this.#copyRequest(newURL, this.request); + } + this.url = newURL; + this.cookies = new AstroCookies(this.request); + this.params = getParams(routeData, this.url.pathname); + this.isRewriting = true; + this.pathname = this.url.pathname; + return await this.render(component); } else { - this.request = this.#copyRequest(newURL, this.request); + this.pipeline.logger.error( + 'router', + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' + ); + return new Response('The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', { + status: 500, + statusText: 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' + }) } - this.url = newURL; - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.isRewriting = true; - this.pathname = this.url.pathname; - return await this.render(component); + }; return { @@ -433,24 +445,36 @@ export class RenderContext { }; const rewrite = async (reroutePayload: RewritePayload) => { - pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; + if (this.pipeline.manifest.rewritingEnabled) { + pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); + const [routeData, component, newURL] = await pipeline.tryRewrite( + reroutePayload, + this.request, + this.originalRoute + ); + this.routeData = routeData; + if (reroutePayload instanceof Request) { + this.request = reroutePayload; + } else { + this.request = this.#copyRequest(newURL, this.request); + } + this.url = new URL(this.request.url); + this.cookies = new AstroCookies(this.request); + this.params = getParams(routeData, this.url.pathname); + this.pathname = this.url.pathname; + this.isRewriting = true; + return await this.render(component); } else { - this.request = this.#copyRequest(newURL, this.request); + this.pipeline.logger.error( + 'router', + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' + ); + return new Response('The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', { + status: 500, + statusText: 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' + }) } - this.url = new URL(this.request.url); - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.pathname = this.url.pathname; - this.isRewriting = true; - return await this.render(component); + }; return { From 9aa98d26b79d2b67c612203c3e41e4e0ce0a67ed Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 26 Jun 2024 14:44:07 +0100 Subject: [PATCH 2/2] apply feedback --- packages/astro/src/core/render-context.ts | 102 +++++++++++----------- packages/astro/test/actions.test.js | 3 + packages/astro/test/astro-cookies.test.js | 3 + 3 files changed, 59 insertions(+), 49 deletions(-) diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index fee594d408f5..93899129d9e5 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -239,35 +239,37 @@ export class RenderContext { const rewrite = async (reroutePayload: RewritePayload) => { pipeline.logger.debug('router', 'Called rewriting to:', reroutePayload); - if (this.pipeline.manifest.rewritingEnabled) { - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; - } else { - this.request = this.#copyRequest(newURL, this.request); - } - this.url = newURL; - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.isRewriting = true; - this.pathname = this.url.pathname; - return await this.render(component); - } else { + if (!this.pipeline.manifest.rewritingEnabled) { this.pipeline.logger.error( 'router', 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' ); - return new Response('The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', { - status: 500, - statusText: 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' - }) + return new Response( + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', + { + status: 500, + statusText: + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', + } + ); } - + const [routeData, component, newURL] = await pipeline.tryRewrite( + reroutePayload, + this.request, + this.originalRoute + ); + this.routeData = routeData; + if (reroutePayload instanceof Request) { + this.request = reroutePayload; + } else { + this.request = this.#copyRequest(newURL, this.request); + } + this.url = newURL; + this.cookies = new AstroCookies(this.request); + this.params = getParams(routeData, this.url.pathname); + this.isRewriting = true; + this.pathname = this.url.pathname; + return await this.render(component); }; return { @@ -445,36 +447,38 @@ export class RenderContext { }; const rewrite = async (reroutePayload: RewritePayload) => { - if (this.pipeline.manifest.rewritingEnabled) { - pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); - const [routeData, component, newURL] = await pipeline.tryRewrite( - reroutePayload, - this.request, - this.originalRoute - ); - this.routeData = routeData; - if (reroutePayload instanceof Request) { - this.request = reroutePayload; - } else { - this.request = this.#copyRequest(newURL, this.request); - } - this.url = new URL(this.request.url); - this.cookies = new AstroCookies(this.request); - this.params = getParams(routeData, this.url.pathname); - this.pathname = this.url.pathname; - this.isRewriting = true; - return await this.render(component); - } else { + if (!this.pipeline.manifest.rewritingEnabled) { this.pipeline.logger.error( 'router', 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' ); - return new Response('The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', { - status: 500, - statusText: 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.' - }) + return new Response( + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', + { + status: 500, + statusText: + 'The rewrite API is experimental. To use this feature, add the `rewriting` flag to the `experimental` object in your Astro config.', + } + ); } - + pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload); + const [routeData, component, newURL] = await pipeline.tryRewrite( + reroutePayload, + this.request, + this.originalRoute + ); + this.routeData = routeData; + if (reroutePayload instanceof Request) { + this.request = reroutePayload; + } else { + this.request = this.#copyRequest(newURL, this.request); + } + this.url = new URL(this.request.url); + this.cookies = new AstroCookies(this.request); + this.params = getParams(routeData, this.url.pathname); + this.pathname = this.url.pathname; + this.isRewriting = true; + return await this.render(component); }; return { diff --git a/packages/astro/test/actions.test.js b/packages/astro/test/actions.test.js index 081e83bf6dd7..e36d29a105e2 100644 --- a/packages/astro/test/actions.test.js +++ b/packages/astro/test/actions.test.js @@ -10,6 +10,9 @@ describe('Astro Actions', () => { fixture = await loadFixture({ root: './fixtures/actions/', adapter: testAdapter(), + experimental: { + rewriting: true + } }); }); diff --git a/packages/astro/test/astro-cookies.test.js b/packages/astro/test/astro-cookies.test.js index 482474b54be2..c27efb243407 100644 --- a/packages/astro/test/astro-cookies.test.js +++ b/packages/astro/test/astro-cookies.test.js @@ -13,6 +13,9 @@ describe('Astro.cookies', () => { root: './fixtures/astro-cookies/', output: 'server', adapter: testAdapter(), + experimental: { + rewriting: true + } }); });