fix(i18n): pass build.format when computing the redirect (#9739)

* fix(i18n): pass `build.format` when computing the direct * Update .changeset/fifty-pots-greet.md Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev> * update tests * add more tests --------- Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com> Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev>
withastro · Jan 22, 2024 · 3ecb3ef · 3ecb3ef
1 parent cfc02c7
commit 3ecb3ef
Show file tree

Hide file tree

Showing 9 changed files with 105 additions and 7 deletions.
diff --git a/.changeset/fifty-pots-greet.md b/.changeset/fifty-pots-greet.md
@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Makes i18n redirects take the `build.format` configuration into account
diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts
@@ -255,7 +255,8 @@ export class App {
 			const i18nMiddleware = createI18nMiddleware(
 				this.#manifest.i18n,
 				this.#manifest.base,
-				this.#manifest.trailingSlash
+				this.#manifest.trailingSlash,
+				this.#manifest.buildFormat
 			);
 			if (i18nMiddleware) {
 				if (mod.onRequest) {

diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts
@@ -40,6 +40,7 @@ export type SSRManifest = {
 	site?: string;
 	base: string;
 	trailingSlash: 'always' | 'never' | 'ignore';
+	buildFormat: 'file' | 'directory';
 	compressHTML: boolean;
 	assetsPrefix?: string;
 	renderers: SSRLoadedRenderer[];

diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -265,7 +265,8 @@ async function generatePage(
 	const i18nMiddleware = createI18nMiddleware(
 		pipeline.getManifest().i18n,
 		pipeline.getManifest().base,
-		pipeline.getManifest().trailingSlash
+		pipeline.getManifest().trailingSlash,
+		pipeline.getManifest().buildFormat
 	);
 	if (config.i18n && i18nMiddleware) {
 		if (onRequest) {
@@ -657,5 +658,6 @@ export function createBuildManifest(
 			: settings.config.site,
 		componentMetadata: internals.componentMetadata,
 		i18n: i18nManifest,
+		buildFormat: settings.config.build.format,
 	};
 }
diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts
@@ -264,5 +264,6 @@ function buildManifest(
 		entryModules,
 		assets: staticFiles.map(prefixAssetPath),
 		i18n: i18nManifest,
+		buildFormat: settings.config.build.format,
 	};
 }
diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts
@@ -2,6 +2,7 @@ import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path';
 import type { Locales, MiddlewareHandler, RouteData, SSRManifest } from '../@types/astro.js';
 import type { PipelineHookFunction } from '../core/pipeline.js';
 import { getPathByLocale, normalizeTheLocale } from './index.js';
+import { shouldAppendForwardSlash } from '../core/build/util.js';
 
 const routeDataSymbol = Symbol.for('astro.routeData');
 
@@ -26,7 +27,8 @@ function pathnameHasLocale(pathname: string, locales: Locales): boolean {
 export function createI18nMiddleware(
 	i18n: SSRManifest['i18n'],
 	base: SSRManifest['base'],
-	trailingSlash: SSRManifest['trailingSlash']
+	trailingSlash: SSRManifest['trailingSlash'],
+	buildFormat: SSRManifest['buildFormat']
 ): MiddlewareHandler | undefined {
 	if (!i18n) {
 		return undefined;
@@ -83,7 +85,7 @@ export function createI18nMiddleware(
 
 				case 'pathname-prefix-always': {
 					if (url.pathname === base + '/' || url.pathname === base) {
-						if (trailingSlash === 'always') {
+						if (shouldAppendForwardSlash(trailingSlash, buildFormat)) {
 							return context.redirect(`${appendForwardSlash(joinPaths(base, i18n.defaultLocale))}`);
 						} else {
 							return context.redirect(`${joinPaths(base, i18n.defaultLocale)}`);

diff --git a/packages/astro/src/vite-plugin-astro-server/plugin.ts b/packages/astro/src/vite-plugin-astro-server/plugin.ts
@@ -125,6 +125,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
 	}
 	return {
 		trailingSlash: settings.config.trailingSlash,
+		buildFormat: settings.config.build.format,
 		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
@@ -308,7 +308,12 @@ export async function handleRoute({
 
 	const onRequest = middleware?.onRequest as MiddlewareHandler | undefined;
 	if (config.i18n) {
-		const i18Middleware = createI18nMiddleware(config.i18n, config.base, config.trailingSlash);
+		const i18Middleware = createI18nMiddleware(
+			config.i18n,
+			config.base,
+			config.trailingSlash,
+			config.build.format
+		);
 
 		if (i18Middleware) {
 			if (onRequest) {

diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js
@@ -229,6 +229,44 @@ describe('[DEV] i18n routing', () => {
 			const response = await fixture.fetch('/new-site/fr/start');
 			expect(response.status).to.equal(404);
 		});
+
+		describe('when `build.format` is `directory`', () => {
+			before(async () => {
+				fixture = await loadFixture({
+					root: './fixtures/i18n-routing-prefix-other-locales/',
+					i18n: {
+						defaultLocale: 'en',
+						locales: [
+							'en',
+							'pt',
+							'it',
+							{
+								path: 'spanish',
+								codes: ['es', 'es-AR'],
+							},
+						],
+						fallback: {
+							it: 'en',
+							spanish: 'en',
+						},
+					},
+					build: {
+						format: 'directory',
+					},
+				});
+				devServer = await fixture.startDevServer();
+			});
+
+			after(async () => {
+				await devServer.stop();
+			});
+
+			it('should redirect to the english locale with trailing slash', async () => {
+				const response = await fixture.fetch('/new-site/it/start/');
+				expect(response.status).to.equal(200);
+				expect(await response.text()).includes('Start');
+			});
+		});
 	});
 
 	describe('i18n routing with routing strategy [pathname-prefix-always-no-redirect]', () => {
@@ -675,6 +713,7 @@ describe('[SSG] i18n routing', () => {
 		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('http-equiv="refresh');
 			expect(html).to.include('url=/new-site/en');
 		});
 
@@ -744,6 +783,25 @@ describe('[SSG] i18n routing', () => {
 				expect(html).to.include('url=/new-site/en');
 			});
 		});
+
+		describe('when `build.format` is `directory`', () => {
+			before(async () => {
+				fixture = await loadFixture({
+					root: './fixtures/i18n-routing-prefix-always/',
+					build: {
+						format: 'directory',
+					},
+				});
+				await fixture.build();
+			});
+
+			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('http-equiv="refresh');
+				expect(html).to.include('url=/new-site/en/');
+			});
+		});
 	});
 
 	describe('i18n routing with fallback', () => {
@@ -940,7 +998,7 @@ describe('[SSR] i18n routing', () => {
 			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');
+			expect(response.headers.get('location')).to.equal('/new-site/en/');
 		});
 
 		it('should render the en locale', async () => {
@@ -1118,7 +1176,7 @@ describe('[SSR] i18n routing', () => {
 			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');
+			expect(response.headers.get('location')).to.equal('/new-site/en/');
 		});
 
 		it('should render the en locale', async () => {
@@ -1173,6 +1231,28 @@ describe('[SSR] i18n routing', () => {
 				expect(response.headers.get('location')).to.equal('/new-site/en/');
 			});
 		});
+
+		describe('when `build.format` is `directory`', () => {
+			before(async () => {
+				fixture = await loadFixture({
+					root: './fixtures/i18n-routing-prefix-always/',
+					output: 'server',
+					adapter: testAdapter(),
+					build: {
+						format: 'directory',
+					},
+				});
+				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', () => {