Skip to content

Commit

Permalink
feat(i18n): expand fallback system (#11677)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
3 people authored Aug 28, 2024
1 parent 1c54e63 commit cb356a5
Show file tree
Hide file tree
Showing 22 changed files with 297 additions and 51 deletions.
22 changes: 22 additions & 0 deletions .changeset/blue-pens-divide.md
Original file line number Diff line number Diff line change
@@ -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")
}
```
32 changes: 32 additions & 0 deletions .changeset/lazy-feet-join.md
Original file line number Diff line number Diff line change
@@ -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"
},
}
})
```


37 changes: 37 additions & 0 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
10 changes: 5 additions & 5 deletions packages/astro/src/container/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand All @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions packages/astro/src/core/app/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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),
Expand All @@ -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> {
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ export type SSRManifest = {

export type SSRManifestI18n = {
fallback: Record<string, string> | undefined;
fallbackType: "redirect" | "rewrite";
strategy: RoutingStrategies;
locales: Locales;
defaultLocale: string;
Expand Down
9 changes: 8 additions & 1 deletion packages/astro/src/core/base-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand All @@ -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
}
3 changes: 2 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions packages/astro/src/core/build/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand All @@ -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> {
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) => {
Expand Down
14 changes: 7 additions & 7 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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,
Expand All @@ -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 {
Expand Down
43 changes: 30 additions & 13 deletions packages/astro/src/core/routing/rewrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
}
}
}
Loading

0 comments on commit cb356a5

Please sign in to comment.