fix(i18n): middleware should consider trailing slash when redirecting (…

…#9085)
withastro · Nov 13, 2023 · fc66ecf · fc66ecf
1 parent 045e5ec
commit fc66ecf
Show file tree

Hide file tree

Showing 9 changed files with 86 additions and 10 deletions.
diff --git a/.changeset/calm-lemons-compare.md 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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', () => {