From 536c6c9fd3d65d1a60bbc8b924c5939f27541d41 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 15 Nov 2023 13:43:57 -0500 Subject: [PATCH] feat(i18n): apply specific routing logic only to pages (#9091) --- .changeset/empty-turtles-wave.md | 5 ++ packages/astro/src/core/app/index.ts | 3 +- packages/astro/src/core/build/generate.ts | 3 +- packages/astro/src/core/pipeline.ts | 20 +++++++ packages/astro/src/i18n/middleware.ts | 27 ++++++++-- .../src/vite-plugin-astro-server/route.ts | 3 +- .../src/pages/test.json.js | 7 +++ packages/astro/test/i18-routing.test.js | 53 +++++++++++++++++++ 8 files changed, 115 insertions(+), 6 deletions(-) create mode 100644 .changeset/empty-turtles-wave.md create mode 100644 packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/test.json.js diff --git a/.changeset/empty-turtles-wave.md b/.changeset/empty-turtles-wave.md new file mode 100644 index 000000000000..3605ca3d645d --- /dev/null +++ b/.changeset/empty-turtles-wave.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +The `routingStrategy` `prefix-always` should not force its logic to endpoints. This fixes some regression with `astro:assets` and `@astrojs/rss`. diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 2d5cd16ed6bb..ec799011907b 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -6,7 +6,7 @@ import type { SSRElement, SSRManifest, } from '../../@types/astro.js'; -import { createI18nMiddleware } from '../../i18n/middleware.js'; +import { createI18nMiddleware, i18nPipelineHook } from '../../i18n/middleware.js'; import type { SinglePageBuiltModule } from '../build/types.js'; import { getSetCookiesFromResponse } from '../cookies/index.js'; import { consoleLogDestination } from '../logger/console.js'; @@ -179,6 +179,7 @@ export class App { } else { this.#pipeline.setMiddlewareFunction(i18nMiddleware); } + this.#pipeline.onBeforeRenderRoute(i18nPipelineHook); } else { if (mod.onRequest) { this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler); diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 6d8f51df2acd..02837cf69cc8 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -30,7 +30,7 @@ import { removeLeadingForwardSlash, removeTrailingForwardSlash, } from '../../core/path.js'; -import { createI18nMiddleware } from '../../i18n/middleware.js'; +import { createI18nMiddleware, i18nPipelineHook } from '../../i18n/middleware.js'; import { runHookBuildGenerated } from '../../integrations/index.js'; import { getOutputDirectory, isServerLikeOutput } from '../../prerender/utils.js'; import { PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js'; @@ -289,6 +289,7 @@ async function generatePage( } else { pipeline.setMiddlewareFunction(i18nMiddleware); } + pipeline.onBeforeRenderRoute(i18nPipelineHook); } else if (onRequest) { pipeline.setMiddlewareFunction(onRequest as MiddlewareEndpointHandler); } diff --git a/packages/astro/src/core/pipeline.ts b/packages/astro/src/core/pipeline.ts index 3d33146c6fa9..bd203b437415 100644 --- a/packages/astro/src/core/pipeline.ts +++ b/packages/astro/src/core/pipeline.ts @@ -15,6 +15,12 @@ type EndpointResultHandler = ( result: Response ) => Promise | Response; +type PipelineHooks = { + before: PipelineHookFunction[]; +}; + +export type PipelineHookFunction = (ctx: RenderContext, mod: ComponentInstance | undefined) => void; + /** * This is the basic class of a pipeline. * @@ -23,6 +29,9 @@ type EndpointResultHandler = ( export class Pipeline { env: Environment; #onRequest?: MiddlewareEndpointHandler; + #hooks: PipelineHooks = { + before: [], + }; /** * The handler accepts the *original* `Request` and result returned by the endpoint. * It must return a `Response`. @@ -75,6 +84,9 @@ export class Pipeline { renderContext: RenderContext, componentInstance: ComponentInstance | undefined ): Promise { + for (const hook of this.#hooks.before) { + hook(renderContext, componentInstance); + } const result = await this.#tryRenderRoute( renderContext, this.env, @@ -158,4 +170,12 @@ export class Pipeline { throw new Error(`Couldn't find route of type [${renderContext.route.type}]`); } } + + /** + * Store a function that will be called before starting the rendering phase. + * @param fn + */ + onBeforeRenderRoute(fn: PipelineHookFunction) { + this.#hooks.before.push(fn); + } } diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index 7d94d79679ea..ff467ecb3131 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -1,5 +1,9 @@ import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path'; -import type { MiddlewareEndpointHandler, SSRManifest } from '../@types/astro.js'; +import type { MiddlewareEndpointHandler, RouteData, SSRManifest } from '../@types/astro.js'; +import type { RouteInfo } from '../core/app/types.js'; +import type { PipelineHookFunction } from '../core/pipeline.js'; + +const routeDataSymbol = Symbol.for('astro.routeData'); // Checks if the pathname doesn't have any locale, exception for the defaultLocale, which is ignored on purpose function checkIsLocaleFree(pathname: string, locales: string[]): boolean { @@ -26,9 +30,19 @@ export function createI18nMiddleware( return await next(); } - const { locales, defaultLocale, fallback } = i18n; - const url = context.url; + const routeData = Reflect.get(context.request, routeDataSymbol); + if (routeData) { + // If the route we're processing is not a page, then we ignore it + if ( + (routeData as RouteData).type !== 'page' && + (routeData as RouteData).type !== 'fallback' + ) { + return await next(); + } + } + const url = context.url; + const { locales, defaultLocale, fallback } = i18n; const response = await next(); if (response instanceof Response) { @@ -83,3 +97,10 @@ export function createI18nMiddleware( return response; }; } + +/** + * This pipeline hook attaches a `RouteData` object to the `Request` + */ +export const i18nPipelineHook: PipelineHookFunction = (ctx) => { + Reflect.set(ctx.request, routeDataSymbol, ctx.route); +}; diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 580ceb0b6468..7468f881907a 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -20,7 +20,7 @@ import { import { createRequest } from '../core/request.js'; import { matchAllRoutes } from '../core/routing/index.js'; import { isPage, resolveIdToUrl } from '../core/util.js'; -import { createI18nMiddleware } from '../i18n/middleware.js'; +import { createI18nMiddleware, i18nPipelineHook } from '../i18n/middleware.js'; import { getSortedPreloadedMatches } from '../prerender/routing.js'; import { isServerLikeOutput } from '../prerender/utils.js'; import { PAGE_SCRIPT_ID } from '../vite-plugin-scripts/index.js'; @@ -289,6 +289,7 @@ export async function handleRoute({ } else { pipeline.setMiddlewareFunction(i18Middleware); } + pipeline.onBeforeRenderRoute(i18nPipelineHook); } else if (onRequest) { pipeline.setMiddlewareFunction(onRequest); } diff --git a/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/test.json.js b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/test.json.js new file mode 100644 index 000000000000..76f3cd4465de --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/test.json.js @@ -0,0 +1,7 @@ +export const GET = () => { + return new Response(JSON.stringify({ lorem: 'ipsum' }), { + headers: { + 'content-type': 'application/json', + }, + }); +}; diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index d9a96ef6eb9f..a7e8b318d371 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -992,3 +992,56 @@ describe('[SSR] i18n routing', () => { }); }); }); + +describe('i18n routing does not break assets and endpoints', () => { + describe('assets', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/core-image-base/', + experimental: { + i18n: { + defaultLocale: 'en', + locales: ['en', 'es'], + }, + }, + base: '/blog', + }); + await fixture.build(); + }); + + it('should render the image', async () => { + const html = await fixture.readFile('/index.html'); + const $ = cheerio.load(html); + const src = $('#local img').attr('src'); + expect(src.length).to.be.greaterThan(0); + expect(src.startsWith('/blog')).to.be.true; + }); + }); + + describe('endpoint', () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + + let app; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-routing-prefix-always/', + output: 'server', + adapter: testAdapter(), + }); + await fixture.build(); + app = await fixture.loadTestAdapterApp(); + }); + + it('should return the expected data', async () => { + let request = new Request('http://example.com/new-site/test.json'); + let response = await app.render(request); + expect(response.status).to.equal(200); + expect(await response.text()).includes('lorem'); + }); + }); +});