From fc66ecff18a20dd436026cb8e75bcc8b5ab0e681 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Mon, 13 Nov 2023 14:12:46 -0500 Subject: [PATCH] fix(i18n): middleware should consider trailing slash when redirecting (#9085) --- .changeset/calm-lemons-compare.md | 5 ++ packages/astro/src/core/app/index.ts | 6 +- packages/astro/src/core/app/types.ts | 1 + packages/astro/src/core/build/generate.ts | 4 +- .../src/core/build/plugins/plugin-manifest.ts | 1 + packages/astro/src/i18n/middleware.ts | 15 +++-- .../src/vite-plugin-astro-server/plugin.ts | 1 + .../src/vite-plugin-astro-server/route.ts | 6 +- packages/astro/test/i18-routing.test.js | 57 ++++++++++++++++++- 9 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 .changeset/calm-lemons-compare.md diff --git a/.changeset/calm-lemons-compare.md b/.changeset/calm-lemons-compare.md new file mode 100644 index 000000000000..d441d65e144e --- /dev/null +++ b/.changeset/calm-lemons-compare.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +When redirecting to the default root locale, Astro middleare should take into consideration the value of `trailingSlash` diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 9c91d06a1ccd..2d5cd16ed6bb 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -166,7 +166,11 @@ export class App { ); let response; try { - let i18nMiddleware = createI18nMiddleware(this.#manifest.i18n, this.#manifest.base); + let i18nMiddleware = createI18nMiddleware( + this.#manifest.i18n, + this.#manifest.base, + this.#manifest.trailingSlash + ); if (i18nMiddleware) { if (mod.onRequest) { this.#pipeline.setMiddlewareFunction( diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 5ae0ecf2c499..9f9d80f44511 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -37,6 +37,7 @@ export type SSRManifest = { routes: RouteInfo[]; site?: string; base: string; + trailingSlash: 'always' | 'never' | 'ignore'; compressHTML: boolean; assetsPrefix?: string; renderers: SSRLoadedRenderer[]; diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts index 0a0bb56448bf..6d8f51df2acd 100644 --- a/packages/astro/src/core/build/generate.ts +++ b/packages/astro/src/core/build/generate.ts @@ -278,7 +278,8 @@ async function generatePage( const onRequest = ssrEntry.onRequest; const i18nMiddleware = createI18nMiddleware( pipeline.getManifest().i18n, - pipeline.getManifest().base + pipeline.getManifest().base, + pipeline.getManifest().trailingSlash ); if (config.experimental.i18n && i18nMiddleware) { if (onRequest) { @@ -636,6 +637,7 @@ export function createBuildManifest( }; } return { + trailingSlash: settings.config.trailingSlash, assets: new Set(), entryModules: Object.fromEntries(internals.entrySpecifierToBundleMap.entries()), routes: [], diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 337719163168..83065ecacbe4 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -256,6 +256,7 @@ function buildManifest( routes, site: settings.config.site, base: settings.config.base, + trailingSlash: settings.config.trailingSlash, compressHTML: settings.config.compressHTML, assetsPrefix: settings.config.build.assetsPrefix, componentMetadata: Array.from(internals.componentMetadata), diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index d50a58b1566f..f40eb2b45b14 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -1,5 +1,5 @@ -import { joinPaths } from '@astrojs/internal-helpers/path'; -import type { MiddlewareEndpointHandler, SSRManifest } from '../@types/astro.js'; +import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path'; +import type { AstroConfig, MiddlewareEndpointHandler, SSRManifest } from '../@types/astro.js'; // 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 { @@ -14,7 +14,8 @@ function checkIsLocaleFree(pathname: string, locales: string[]): boolean { export function createI18nMiddleware( i18n: SSRManifest['i18n'], - base: SSRManifest['base'] + base: SSRManifest['base'], + trailingSlash: SSRManifest['trailingSlash'] ): MiddlewareEndpointHandler | undefined { if (!i18n) { return undefined; @@ -42,8 +43,12 @@ export function createI18nMiddleware( headers: response.headers, }); } else if (i18n.routingStrategy === 'prefix-always') { - if (url.pathname === base || url.pathname === base + '/') { - return context.redirect(`${joinPaths(base, i18n.defaultLocale)}`); + if (url.pathname === base + '/' || url.pathname === base) { + if (trailingSlash === 'always') { + return context.redirect(`${appendForwardSlash(joinPaths(base, i18n.defaultLocale))}`); + } else { + return context.redirect(`${joinPaths(base, i18n.defaultLocale)}`); + } } // Astro can't know where the default locale is supposed to be, so it returns a 404 with no content. diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts index 94ac24c9c924..b8f4ab661aba 100644 --- a/packages/astro/src/vite-plugin-astro-server/plugin.ts +++ b/packages/astro/src/vite-plugin-astro-server/plugin.ts @@ -96,6 +96,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest }; } return { + trailingSlash: settings.config.trailingSlash, compressHTML: settings.config.compressHTML, assets: new Set(), entryModules: {}, diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 89173a1ecf8d..580ceb0b6468 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -277,7 +277,11 @@ export async function handleRoute({ const onRequest = middleware?.onRequest as MiddlewareEndpointHandler | undefined; if (config.experimental.i18n) { - const i18Middleware = createI18nMiddleware(config.experimental.i18n, config.base); + const i18Middleware = createI18nMiddleware( + config.experimental.i18n, + config.base, + config.trailingSlash + ); if (i18Middleware) { if (onRequest) { diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index 47d6b65af05f..d9a96ef6eb9f 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -253,6 +253,26 @@ describe('[DEV] i18n routing', () => { const response = await fixture.fetch('/new-site/fr/start'); expect(response.status).to.equal(404); }); + + describe('[trailingSlash: always]', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-routing-prefix-always/', + trailingSlash: 'always', + }); + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }); + + it('should redirect to the index of the default locale', async () => { + const response = await fixture.fetch('/new-site/'); + expect(response.status).to.equal(200); + expect(await response.text()).includes('Hello'); + }); + }); }); describe('i18n routing fallback', () => { @@ -314,7 +334,6 @@ describe('[DEV] i18n routing', () => { }); }); }); - describe('[SSG] i18n routing', () => { describe('i18n routing', () => { /** @type {import('./test-utils').Fixture} */ @@ -547,6 +566,21 @@ describe('[SSG] i18n routing', () => { return true; } }); + + describe('[trailingSlash: always]', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-routing-prefix-always/', + trailingSlash: 'always', + }); + }); + + it('should redirect to the index of the default locale', async () => { + const html = await fixture.readFile('/index.html'); + expect(html).to.include('http-equiv="refresh'); + expect(html).to.include('url=/new-site/en'); + }); + }); }); describe('i18n routing with fallback', () => { @@ -607,7 +641,6 @@ describe('[SSG] i18n routing', () => { }); }); }); - describe('[SSR] i18n routing', () => { let app; describe('default', () => { @@ -792,6 +825,26 @@ describe('[SSR] i18n routing', () => { let response = await app.render(request); expect(response.status).to.equal(404); }); + + describe('[trailingSlash: always]', () => { + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-routing-prefix-always/', + output: 'server', + adapter: testAdapter(), + trailingSlash: 'always', + }); + await fixture.build(); + app = await fixture.loadTestAdapterApp(); + }); + + it('should redirect to the index of the default locale', async () => { + let request = new Request('http://example.com/new-site/'); + let response = await app.render(request); + expect(response.status).to.equal(302); + expect(response.headers.get('location')).to.equal('/new-site/en/'); + }); + }); }); describe('with fallback', () => {