feat(i18n): expand fallback system (#11677)

* feat(i18n): expand fallback system * rebase * apply feedback * better changeset * update tests too * apply feedback * Update .changeset/blue-pens-divide.md Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> * update docs * nitpick * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update .changeset/blue-pens-divide.md Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update .changeset/blue-pens-divide.md Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * fix regression --------- Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca>
withastro · Aug 28, 2024 · cb356a5 · cb356a5
1 parent 1c54e63
commit cb356a5
Show file tree

Hide file tree

Showing 22 changed files with 297 additions and 51 deletions.
diff --git a/.changeset/blue-pens-divide.md b/.changeset/blue-pens-divide.md
@@ -0,0 +1,22 @@
+---
+'astro': patch
+---
+
+Fixes a bug in the logic of `Astro.rewrite()` which led to the value for `base`, if configured, being automatically prepended to the rewrite URL passed. This was unintended behavior and has been corrected, and Astro now processes the URLs exactly as passed.
+
+If you use the `rewrite()` function on a project that has `base` configured, you must now prepend the base to your existing rewrite URL:
+
+```js
+// astro.config.mjs
+export default defineConfig({
+  base: '/blog'
+})
+```
+
+```diff
+// src/middleware.js
+export function onRequest(ctx, next) {
+-  return ctx.rewrite("/about")
++  return ctx.rewrite("/blog/about")
+}
+```
diff --git a/.changeset/lazy-feet-join.md b/.changeset/lazy-feet-join.md
@@ -0,0 +1,32 @@
+---
+'astro': minor
+---
+
+Adds a new option `fallbackType` to `i18n.routing` configuration that allows you to control how fallback pages are handled.
+
+When `i18n.fallback` is configured, this new routing option controls whether to [redirect](https://docs.astro.build/en/guides/routing/#redirects) to the fallback page, or to [rewrite](https://docs.astro.build/en/guides/routing/#rewrites) the fallback page's content in place.
+
+The `"redirect"` option is the default value and matches the current behavior of the existing fallback system.
+
+The option `"rewrite"` uses the new [rewriting system](https://docs.astro.build/en/guides/routing/#rewrites) to create fallback pages that render content on the original, requested URL without a browser refresh.
+
+For example, the following configuration will generate a page `/fr/index.html` that will contain the same HTML rendered by the page `/en/index.html` when `src/pages/fr/index.astro` does not exist.
+
+```js
+// astro.config.mjs
+export default defineConfig({
+  i18n: {
+    locals: ["en", "fr"],
+    defaultLocale: "en",
+    routing: {
+      prefixDefaultLocale: true,
+      fallbackType: "rewrite"
+    },
+    fallback: {
+      fr: "en"
+    },
+  }
+})
+```
+
+
diff --git a/packages/astro/src/@types/astro.ts b/packages/astro/src/@types/astro.ts
@@ -1668,6 +1668,43 @@ export interface AstroUserConfig {
 					 * */
 					redirectToDefaultLocale?: boolean;
 
+					/**
+					 * @docs
+					 * @name i18n.routing.fallbackType
+					 * @kind h4
+					 * @type {"redirect" | "rewrite"}
+					 * @default `"redirect"`
+					 * @version 4.15.0
+					 * @description
+					 *
+					 * When [`i18n.fallback`](#i18nfallback) is configured to avoid showing a 404 page for missing page routes, this option controls whether to [redirect](https://docs.astro.build/en/guides/routing/#redirects) to the fallback page, or to [rewrite](https://docs.astro.build/en/guides/routing/#rewrites) the fallback page's content in place.
+					 * 
+					 * By default, Astro's i18n routing creates pages that redirect your visitors to a new destination based on your fallback configuration. The browser will refresh and show the destination address in the URL bar.
+					 *
+					 * When `i18n.routing.fallback: "rewrite"` is configured, Astro will create pages that render the contents of the fallback page on the original, requested URL.
+					 *
+					 * With the following configuration, if you have the file `src/pages/en/about.astro` but not `src/pages/fr/about.astro`, the `astro build` command will generate `dist/fr/about.html` with the same content as the `dist/en/index.html` page.
+					 * Your site visitor will see the English version of the page at `https://example.com/fr/about/` and will not be redirected.
+					 * 
+					 * ```js
+					 * //astro.config.mjs
+					 * export default defineConfig({
+					 * 	 i18n: {
+					 *     defaultLocale: "en",
+					 *     locales: ["en", "fr"],
+					 *     routing: {
+					 *     	prefixDefaultLocale: false,
+					 *     	fallbackType: "rewrite",
+					 *     },
+					 *     fallback: {
+					 *     	fr: "en",
+					 *     }
+					 *   },
+					 * })
+					 * ```
+					 */
+					fallbackType: "redirect" | "rewrite"
+
 					/**
 					 * @name i18n.routing.strategy
 					 * @type {"pathname"}

diff --git a/packages/astro/src/container/pipeline.ts b/packages/astro/src/container/pipeline.ts
@@ -5,7 +5,7 @@ import type {
 	SSRElement,
 	SSRResult,
 } from '../@types/astro.js';
-import { type HeadElements, Pipeline } from '../core/base-pipeline.js';
+import {type HeadElements, Pipeline, type TryRewriteResult} from '../core/base-pipeline.js';
 import type { SinglePageBuiltModule } from '../core/build/types.js';
 import {
 	createModuleScriptElement,
@@ -71,8 +71,8 @@ export class ContainerPipeline extends Pipeline {
 	async tryRewrite(
 		payload: RewritePayload,
 		request: Request,
-	): Promise<[RouteData, ComponentInstance, URL]> {
-		const [foundRoute, finalUrl] = findRouteToRewrite({
+	): Promise<TryRewriteResult> {
+		const {newUrl,pathname,routeData} = findRouteToRewrite({
 			payload,
 			request,
 			routes: this.manifest?.routes.map((r) => r.routeData),
@@ -81,8 +81,8 @@ export class ContainerPipeline extends Pipeline {
 			base: this.manifest.base,
 		});
 
-		const componentInstance = await this.getComponentByRoute(foundRoute);
-		return [foundRoute, componentInstance, finalUrl];
+		const componentInstance = await this.getComponentByRoute(routeData);
+		return {componentInstance, routeData, newUrl, pathname};
 	}
 
 	insertRoute(route: RouteData, componentInstance: ComponentInstance): void {

diff --git a/packages/astro/src/core/app/pipeline.ts b/packages/astro/src/core/app/pipeline.ts
@@ -6,7 +6,7 @@ import type {
 	SSRElement,
 	SSRResult,
 } from '../../@types/astro.js';
-import { Pipeline } from '../base-pipeline.js';
+import {Pipeline, type TryRewriteResult} from '../base-pipeline.js';
 import type { SinglePageBuiltModule } from '../build/types.js';
 import { RedirectSinglePageBuiltModule } from '../redirects/component.js';
 import { createModuleScriptElement, createStylesheetElementSet } from '../render/ssr-element.js';
@@ -94,8 +94,8 @@ export class AppPipeline extends Pipeline {
 		payload: RewritePayload,
 		request: Request,
 		_sourceRoute: RouteData,
-	): Promise<[RouteData, ComponentInstance, URL]> {
-		const [foundRoute, finalUrl] = findRouteToRewrite({
+	): Promise<TryRewriteResult> {
+		const { newUrl,pathname,routeData} = findRouteToRewrite({
 			payload,
 			request,
 			routes: this.manifest?.routes.map((r) => r.routeData),
@@ -104,8 +104,8 @@ export class AppPipeline extends Pipeline {
 			base: this.manifest.base,
 		});
 
-		const componentInstance = await this.getComponentByRoute(foundRoute);
-		return [foundRoute, componentInstance, finalUrl];
+		const componentInstance = await this.getComponentByRoute(routeData);
+		return {newUrl, pathname, componentInstance, routeData};
 	}
 
 	async getModuleForRoute(route: RouteData): Promise<SinglePageBuiltModule> {

diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts
@@ -76,6 +76,7 @@ export type SSRManifest = {
 
 export type SSRManifestI18n = {
 	fallback: Record<string, string> | undefined;
+	fallbackType: "redirect" | "rewrite";
 	strategy: RoutingStrategies;
 	locales: Locales;
 	defaultLocale: string;

diff --git a/packages/astro/src/core/base-pipeline.ts b/packages/astro/src/core/base-pipeline.ts
@@ -95,7 +95,7 @@ export abstract class Pipeline {
 		rewritePayload: RewritePayload,
 		request: Request,
 		sourceRoute: RouteData,
-	): Promise<[RouteData, ComponentInstance, URL]>;
+	): Promise<TryRewriteResult>;
 
 	/**
 	 * Tells the pipeline how to retrieve a component give a `RouteData`
@@ -106,3 +106,10 @@ export abstract class Pipeline {
 
 // eslint-disable-next-line @typescript-eslint/no-empty-object-type
 export interface HeadElements extends Pick<SSRResult, 'scripts' | 'styles' | 'links'> {}
+
+export interface TryRewriteResult {
+	routeData: RouteData, 
+	componentInstance: ComponentInstance,
+	newUrl: URL,
+	pathname: string
+}
diff --git a/packages/astro/src/core/build/generate.ts b/packages/astro/src/core/build/generate.ts
@@ -26,7 +26,7 @@ import {
 	removeLeadingForwardSlash,
 	removeTrailingForwardSlash,
 } from '../../core/path.js';
-import { toRoutingStrategy } from '../../i18n/utils.js';
+import {toFallbackType, toRoutingStrategy} from '../../i18n/utils.js';
 import { runHookBuildGenerated } from '../../integrations/hooks.js';
 import { getOutputDirectory } from '../../prerender/utils.js';
 import type { SSRManifestI18n } from '../app/types.js';
@@ -528,6 +528,7 @@ function createBuildManifest(
 	if (settings.config.i18n) {
 		i18nManifest = {
 			fallback: settings.config.i18n.fallback,
+			fallbackType: toFallbackType(settings.config.i18n.routing),
 			strategy: toRoutingStrategy(settings.config.i18n.routing, settings.config.i18n.domains),
 			defaultLocale: settings.config.i18n.defaultLocale,
 			locales: settings.config.i18n.locales,

diff --git a/packages/astro/src/core/build/pipeline.ts b/packages/astro/src/core/build/pipeline.ts
@@ -26,6 +26,7 @@ import { RESOLVED_SPLIT_MODULE_ID } from './plugins/plugin-ssr.js';
 import { getPagesFromVirtualModulePageName, getVirtualModulePageName } from './plugins/util.js';
 import type { PageBuildData, SinglePageBuiltModule, StaticBuildOptions } from './types.js';
 import { i18nHasFallback } from './util.js';
+import type {TryRewriteResult} from "../base-pipeline.js";
 
 /**
  * The build pipeline is responsible to gather the files emitted by the SSR build and generate the pages by executing these files.
@@ -289,8 +290,8 @@ export class BuildPipeline extends Pipeline {
 		payload: RewritePayload,
 		request: Request,
 		_sourceRoute: RouteData,
-	): Promise<[RouteData, ComponentInstance, URL]> {
-		const [foundRoute, finalUrl] = findRouteToRewrite({
+	): Promise<TryRewriteResult> {
+		const { routeData, pathname, newUrl} = findRouteToRewrite({
 			payload,
 			request,
 			routes: this.options.manifest.routes,
@@ -299,8 +300,8 @@ export class BuildPipeline extends Pipeline {
 			base: this.config.base,
 		});
 
-		const componentInstance = await this.getComponentByRoute(foundRoute);
-		return [foundRoute, componentInstance, finalUrl];
+		const componentInstance = await this.getComponentByRoute(routeData);
+		return { routeData, componentInstance, newUrl, pathname };
 	}
 
 	async retrieveSsrEntry(route: RouteData, filePath: string): Promise<SinglePageBuiltModule> {

diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts
@@ -4,7 +4,7 @@ import type { OutputChunk } from 'rollup';
 import type { Plugin as VitePlugin } from 'vite';
 import { getAssetsPrefix } from '../../../assets/utils/getAssetsPrefix.js';
 import { normalizeTheLocale } from '../../../i18n/index.js';
-import { toRoutingStrategy } from '../../../i18n/utils.js';
+import {toFallbackType, toRoutingStrategy} from '../../../i18n/utils.js';
 import { runHookBuildSsr } from '../../../integrations/hooks.js';
 import { BEFORE_HYDRATION_SCRIPT_ID, PAGE_SCRIPT_ID } from '../../../vite-plugin-scripts/index.js';
 import type {
@@ -254,6 +254,7 @@ function buildManifest(
 	if (settings.config.i18n) {
 		i18nManifest = {
 			fallback: settings.config.i18n.fallback,
+			fallbackType: toFallbackType(settings.config.i18n.routing),
 			strategy: toRoutingStrategy(settings.config.i18n.routing, settings.config.i18n.domains),
 			locales: settings.config.i18n.locales,
 			defaultLocale: settings.config.i18n.defaultLocale,

diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts
@@ -403,6 +403,7 @@ export const AstroConfigSchema = z.object({
 							.object({
 								prefixDefaultLocale: z.boolean().optional().default(false),
 								redirectToDefaultLocale: z.boolean().optional().default(true),
+								fallbackType: z.enum(["redirect", "rewrite"]).optional().default("redirect"),
 							})
 							.refine(
 								({ prefixDefaultLocale, redirectToDefaultLocale }) => {

diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts
@@ -142,13 +142,13 @@ export class RenderContext {
 			if (payload) {
 				pipeline.logger.debug('router', 'Called rewriting to:', payload);
 				// we intentionally let the error bubble up
-				const [routeData, component] = await pipeline.tryRewrite(
+				const { routeData, componentInstance: newComponent} = await pipeline.tryRewrite(
 					payload,
 					this.request,
 					this.originalRoute,
 				);
 				this.routeData = routeData;
-				componentInstance = component;
+				componentInstance = newComponent;
 				this.isRewriting = true;
 				this.status = 200;
 			}
@@ -234,7 +234,7 @@ export class RenderContext {
 
 	async #executeRewrite(reroutePayload: RewritePayload) {
 		this.pipeline.logger.debug('router', 'Calling rewrite: ', reroutePayload);
-		const [routeData, component, newURL] = await this.pipeline.tryRewrite(
+		const { routeData, componentInstance, newUrl, pathname } = await this.pipeline.tryRewrite(
 			reroutePayload,
 			this.request,
 			this.originalRoute,
@@ -243,16 +243,16 @@ export class RenderContext {
 		if (reroutePayload instanceof Request) {
 			this.request = reroutePayload;
 		} else {
-			this.request = this.#copyRequest(newURL, this.request);
+			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.params = getParams(routeData, pathname);
+		this.pathname = pathname;
 		this.isRewriting = true;
 		// we found a route and a component, we can change the status code to 200
 		this.status = 200;
-		return await this.render(component);
+		return await this.render(componentInstance);
 	}
 
 	createActionAPIContext(): ActionAPIContext {

diff --git a/packages/astro/src/core/routing/rewrite.ts b/packages/astro/src/core/routing/rewrite.ts
@@ -12,44 +12,61 @@ export type FindRouteToRewrite = {
 	base: AstroConfig['base'];
 };
 
+export interface FindRouteToRewriteResult {
+	routeData: RouteData;
+	newUrl: URL;
+	pathname: string;
+}
+
+/**
+ * Shared logic to retrieve the rewritten route. It returns a tuple that represents:
+ * 1. The new `Request` object. It contains `base`
+ * 2.
+ */
 export function findRouteToRewrite({
 	payload,
 	routes,
 	request,
 	trailingSlash,
 	buildFormat,
 	base,
-}: FindRouteToRewrite): [RouteData, URL] {
-	let finalUrl: URL | undefined = undefined;
+}: FindRouteToRewrite): FindRouteToRewriteResult {
+	let newUrl: URL | undefined = undefined;
 	if (payload instanceof URL) {
-		finalUrl = payload;
+		newUrl = payload;
 	} else if (payload instanceof Request) {
-		finalUrl = new URL(payload.url);
+		newUrl = new URL(payload.url);
 	} else {
-		finalUrl = new URL(payload, new URL(request.url).origin);
+		newUrl = new URL(payload, new URL(request.url).origin);
+	}
+	let pathname = newUrl.pathname;
+	if (base !== '/' && newUrl.pathname.startsWith(base)) {
+		pathname = shouldAppendForwardSlash(trailingSlash, buildFormat)
+			? appendForwardSlash(newUrl.pathname)
+			: removeTrailingForwardSlash(newUrl.pathname);
+		pathname = pathname.slice(base.length);
 	}
 
 	let foundRoute;
 	for (const route of routes) {
-		const pathname = shouldAppendForwardSlash(trailingSlash, buildFormat)
-			? appendForwardSlash(finalUrl.pathname)
-			: base !== '/'
-				? removeTrailingForwardSlash(finalUrl.pathname)
-				: finalUrl.pathname;
 		if (route.pattern.test(decodeURI(pathname))) {
 			foundRoute = route;
 			break;
 		}
 	}
 
 	if (foundRoute) {
-		return [foundRoute, finalUrl];
+		return {
+			routeData: foundRoute,
+			newUrl, 
+			pathname
+		};
 	} else {
 		const custom404 = routes.find((route) => route.route === '/404');
 		if (custom404) {
-			return [custom404, finalUrl];
+			return { routeData: custom404, newUrl, pathname };
 		} else {
-			return [DEFAULT_404_ROUTE, finalUrl];
+			return {  routeData: DEFAULT_404_ROUTE, newUrl, pathname };
 		}
 	}
 }