From 0529ac122ab609a27136ef5c16f69c8df3a9da44 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Tue, 24 Oct 2023 11:07:27 +0100 Subject: [PATCH] feat(i18n): fallback logic of defaultLocale for i18n routing (#8899) --- packages/astro/src/core/app/types.ts | 2 +- packages/astro/src/core/build/page-data.ts | 1 + packages/astro/src/core/config/schema.ts | 20 ++- packages/astro/src/core/render/core.ts | 7 +- .../astro/src/core/routing/manifest/create.ts | 136 +++++++++++++----- packages/astro/src/i18n/middleware.ts | 44 ++++-- .../i18n-routing-fallback/astro.config.mjs | 3 +- .../src/pages/{en => }/blog/[id].astro | 0 .../i18n-routing-fallback/src/pages/end.astro | 8 ++ .../src/pages/pt/start.astro | 2 +- .../src/pages/{en => }/start.astro | 2 +- packages/astro/test/i18-routing.test.js | 38 ++--- .../test/units/config/config-validate.test.js | 4 +- 13 files changed, 179 insertions(+), 88 deletions(-) rename packages/astro/test/fixtures/i18n-routing-fallback/src/pages/{en => }/blog/[id].astro (100%) create mode 100644 packages/astro/test/fixtures/i18n-routing-fallback/src/pages/end.astro rename packages/astro/test/fixtures/i18n-routing-fallback/src/pages/{en => }/start.astro (91%) diff --git a/packages/astro/src/core/app/types.ts b/packages/astro/src/core/app/types.ts index 4c25ca97e254..4c95c6c0d600 100644 --- a/packages/astro/src/core/app/types.ts +++ b/packages/astro/src/core/app/types.ts @@ -53,7 +53,7 @@ export type SSRManifest = { }; export type SSRManifestI18n = { - fallback?: Record; + fallback?: Record; fallbackControl?: 'none' | 'redirect' | 'render'; locales: string[]; defaultLocale: string; diff --git a/packages/astro/src/core/build/page-data.ts b/packages/astro/src/core/build/page-data.ts index 7292cb4e8f94..f4426191a117 100644 --- a/packages/astro/src/core/build/page-data.ts +++ b/packages/astro/src/core/build/page-data.ts @@ -4,6 +4,7 @@ import type { AllPagesData } from './types.js'; import * as colors from 'kleur/colors'; import { debug } from '../logger/core.js'; +import { eachPageFromAllPages } from './internal.js'; export interface CollectPagesDataOptions { settings: AstroSettings; diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 57e26f8bdbdd..b09f772a09db 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -304,7 +304,7 @@ export const AstroConfigSchema = z.object({ .object({ defaultLocale: z.string(), locales: z.string().array(), - fallback: z.record(z.string(), z.string().array()).optional(), + fallback: z.record(z.string(), z.string()).optional(), detectBrowserLanguage: z.boolean().optional().default(false), // TODO: properly add default when the feature goes of experimental fallbackControl: z.enum(['none', 'redirect', 'render']).optional(), @@ -320,21 +320,19 @@ export const AstroConfigSchema = z.object({ }); } if (fallback) { - for (const [fallbackKey, fallbackArray] of Object.entries(fallback)) { - if (!locales.includes(fallbackKey)) { + for (const [fallbackFrom, fallbackTo] of Object.entries(fallback)) { + if (!locales.includes(fallbackFrom)) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: `The locale \`${fallbackKey}\` key in the \`i18n.fallback\` record doesn't exist in the \`i18n.locales\` array.`, + message: `The locale \`${fallbackFrom}\` key in the \`i18n.fallback\` record doesn't exist in the \`i18n.locales\` array.`, }); } - for (const fallbackArrayKey of fallbackArray) { - if (!locales.includes(fallbackArrayKey)) { - ctx.addIssue({ - code: z.ZodIssueCode.custom, - message: `The locale \`${fallbackArrayKey}\` value in the \`i18n.fallback\` record doesn't exist in the \`i18n.locales\` array.`, - }); - } + if (!locales.includes(fallbackTo)) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + message: `The locale \`${fallbackTo}\` value in the \`i18n.fallback\` record doesn't exist in the \`i18n.locales\` array.`, + }); } } } diff --git a/packages/astro/src/core/render/core.ts b/packages/astro/src/core/render/core.ts index 5413f716fe35..d10ef0c667cd 100644 --- a/packages/astro/src/core/render/core.ts +++ b/packages/astro/src/core/render/core.ts @@ -32,9 +32,12 @@ export async function renderPage({ mod, renderContext, env, cookies }: RenderPag location: redirectRouteGenerate(renderContext.route, renderContext.params), }, }); - // TODO: check this one } else if (routeIsFallback(renderContext.route)) { - return new Response(null); + // We return a 404 because fallback routes don't exist. + // It's responsibility of the middleware to catch them and re-route the requests + return new Response(null, { + status: 404, + }); } else if (!mod) { throw new AstroError(CantRenderPage); } diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index eb24b01ac35a..b3789404d53c 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -484,50 +484,110 @@ export function createRouteManifest( // Didn't find a good place, insert last routes.push(routeData); }); + const i18n = settings.config.experimental.i18n; + + if (i18n && i18n.fallback && i18n.fallbackControl === 'redirect') { + let fallback = Object.entries(i18n.fallback); + + // A map like: locale => RouteData[] + const routesByLocale = new Map(); + // We create a set, so we can remove the routes that have been added to the previous map + const setRoutes = new Set(routes); + + // First loop + // We loop over the locales minus the default locale and add only the routes that contain `/`. + for (const locale of i18n.locales.filter((loc) => loc !== i18n.defaultLocale)) { + for (const route of setRoutes) { + if (!route.route.includes(`/${locale}`)) { + continue; + } + const currentRoutes = routesByLocale.get(locale); + if (currentRoutes) { + currentRoutes.push(route); + routesByLocale.set(locale, currentRoutes); + } else { + routesByLocale.set(locale, [route]); + } + setRoutes.delete(route); + } + } + + // we loop over the remaining routes and add them to the default locale + for (const route of setRoutes) { + const currentRoutes = routesByLocale.get(i18n.defaultLocale); + if (currentRoutes) { + currentRoutes.push(route); + routesByLocale.set(i18n.defaultLocale, currentRoutes); + } else { + routesByLocale.set(i18n.defaultLocale, [route]); + } + setRoutes.delete(route); + } - if (settings.config.experimental.i18n && settings.config.experimental.i18n.fallback) { - let fallback = Object.entries(settings.config.experimental.i18n.fallback); if (fallback.length > 0) { - for (const [fallbackLocale, fallbackLocaleList] of fallback) { - for (const fallbackLocaleEntry of fallbackLocaleList) { - const fallbackToRoutes = routes.filter((r) => - r.component.includes(`/${fallbackLocaleEntry}`) - ); - const fallbackFromRoutes = routes.filter((r) => - r.component.includes(`/${fallbackLocale}`) - ); - - for (const fallbackToRoute of fallbackToRoutes) { - const hasRoute = fallbackFromRoutes.some((r) => - r.component.replace(`/${fallbackLocaleEntry}`, `/${fallbackLocale}`) - ); - - if (!hasRoute) { - const pathname = fallbackToRoute.pathname?.replace( - `/${fallbackLocaleEntry}`, - `/${fallbackLocale}` + for (const [fallbackFromLocale, fallbackToLocale] of fallback) { + let fallbackToRoutes; + if (fallbackToLocale === i18n.defaultLocale) { + fallbackToRoutes = routesByLocale.get(i18n.defaultLocale); + } else { + fallbackToRoutes = routesByLocale.get(fallbackToLocale); + } + const fallbackFromRoutes = routesByLocale.get(fallbackFromLocale); + + // Technically, we should always have a fallback to. Added this to make TS happy. + if (!fallbackToRoutes) { + continue; + } + + for (const fallbackToRoute of fallbackToRoutes) { + const hasRoute = + fallbackFromRoutes && + // we check if the fallback from locale (the origin) has already this route + fallbackFromRoutes.some((route) => { + if (fallbackToLocale === i18n.defaultLocale) { + return route.route.replace(`/${fallbackFromLocale}`, '') === fallbackToRoute.route; + } else { + return ( + route.route.replace(`/${fallbackToLocale}`, `/${fallbackFromLocale}`) === + fallbackToRoute.route + ); + } + }); + + if (!hasRoute) { + let pathname: string | undefined; + let route: string; + if (fallbackToLocale === i18n.defaultLocale) { + if (fallbackToRoute.pathname) { + pathname = `/${fallbackFromLocale}${fallbackToRoute.pathname}`; + } + route = `/${fallbackFromLocale}${fallbackToRoute.route}`; + } else { + pathname = fallbackToRoute.pathname?.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` ); - const route = fallbackToRoute.route?.replace( - `/${fallbackLocaleEntry}`, - `/${fallbackLocale}` + route = fallbackToRoute.route.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` ); + } - const segments = removeLeadingForwardSlash(route) - .split(path.posix.sep) - .filter(Boolean) - .map((s: string) => { - validateSegment(s); - return getParts(s, route); - }); - routes.push({ - ...fallbackToRoute, - pathname, - route, - segments, - pattern: getPattern(segments, config), - type: 'fallback', + const segments = removeLeadingForwardSlash(route) + .split(path.posix.sep) + .filter(Boolean) + .map((s: string) => { + validateSegment(s); + return getParts(s, route); }); - } + routes.push({ + ...fallbackToRoute, + pathname, + route, + segments, + pattern: getPattern(segments, config), + type: 'fallback', + }); } } } diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index e876a3c990d8..4d6993713870 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -1,5 +1,6 @@ import type { MiddlewareEndpointHandler } from '../@types/astro.js'; import type { SSRManifest } from '../@types/astro.js'; +import type { Environment } from '../core/render/index.js'; export function createI18nMiddleware( i18n: SSRManifest['i18n'] @@ -10,23 +11,42 @@ export function createI18nMiddleware( const locales = i18n.locales; return async (context, next) => { - if (!i18n.fallback) { - return next(); + if (!i18n.fallback || !i18n.fallback) { + return await next(); } - const response = await next(); - if (i18n.fallbackControl === 'redirect' && response instanceof Response) { - const fallbackKeys = i18n.fallback ? Object.keys(i18n.fallback) : []; - const url = context.url; + + const url = context.url; + if (response instanceof Response) { const separators = url.pathname.split('/'); + const fallbackWithRedirect = i18n.fallbackControl === 'redirect'; + if (fallbackWithRedirect && url.pathname.includes(`/${i18n.defaultLocale}`)) { + const content = await response.text(); + const newLocation = url.pathname.replace(`/${i18n.defaultLocale}`, ''); + response.headers.set('Location', newLocation); + return new Response(content, { + status: 302, + headers: response.headers, + }); + } + if (fallbackWithRedirect && response.status >= 300) { + const fallbackKeys = i18n.fallback ? Object.keys(i18n.fallback) : []; + + const urlLocale = separators.find((s) => locales.includes(s)); - const urlLocale = separators.find((s) => locales.includes(s)); + if (urlLocale && fallbackKeys.includes(urlLocale)) { + const fallbackLocale = i18n.fallback[urlLocale]; + let newPathname: string; + // If a locale falls back to the default locale, we want to **remove** the locale because + // the default locale doesn't have a prefix + if (fallbackLocale === i18n.defaultLocale) { + newPathname = url.pathname.replace(`/${urlLocale}`, ``); + } else { + newPathname = url.pathname.replace(`/${urlLocale}`, `/${fallbackLocale}`); + } - if (urlLocale && fallbackKeys.includes(urlLocale)) { - // TODO: correctly handle chain of fallback - const fallbackLocale = i18n.fallback[urlLocale][0]; - const newPathname = url.pathname.replace(`/${urlLocale}`, `/${fallbackLocale}`); - return context.redirect(newPathname); + return context.redirect(newPathname); + } } } diff --git a/packages/astro/test/fixtures/i18n-routing-fallback/astro.config.mjs b/packages/astro/test/fixtures/i18n-routing-fallback/astro.config.mjs index a1a3bbe5b32d..f7524a64210d 100644 --- a/packages/astro/test/fixtures/i18n-routing-fallback/astro.config.mjs +++ b/packages/astro/test/fixtures/i18n-routing-fallback/astro.config.mjs @@ -9,7 +9,8 @@ export default defineConfig({ 'en', 'pt', 'it' ], fallback: { - "it": ["en"] + "it": "en", + pt: "en" } } } diff --git a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/en/blog/[id].astro b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/blog/[id].astro similarity index 100% rename from packages/astro/test/fixtures/i18n-routing-fallback/src/pages/en/blog/[id].astro rename to packages/astro/test/fixtures/i18n-routing-fallback/src/pages/blog/[id].astro diff --git a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/end.astro b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/end.astro new file mode 100644 index 000000000000..9f33d8aa0bd3 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/end.astro @@ -0,0 +1,8 @@ + + + Astro + + +End + + diff --git a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/pt/start.astro b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/pt/start.astro index 15a63a7b87f5..5a4a84c2cf0c 100644 --- a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/pt/start.astro +++ b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/pt/start.astro @@ -3,6 +3,6 @@ Astro -Hola +Oi essa e start diff --git a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/en/start.astro b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/start.astro similarity index 91% rename from packages/astro/test/fixtures/i18n-routing-fallback/src/pages/en/start.astro rename to packages/astro/test/fixtures/i18n-routing-fallback/src/pages/start.astro index d9f61aa025c1..990baecd9a8c 100644 --- a/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/en/start.astro +++ b/packages/astro/test/fixtures/i18n-routing-fallback/src/pages/start.astro @@ -3,6 +3,6 @@ Astro -Hello +Start diff --git a/packages/astro/test/i18-routing.test.js b/packages/astro/test/i18-routing.test.js index dff4b7c77858..e9fbf9187d21 100644 --- a/packages/astro/test/i18-routing.test.js +++ b/packages/astro/test/i18-routing.test.js @@ -114,7 +114,7 @@ describe('[DEV] i18n routing', () => { defaultLocale: 'en', locales: ['en', 'pt', 'it'], fallback: { - it: ['en'], + it: 'en', }, fallbackControl: 'redirect', }, @@ -127,12 +127,12 @@ describe('[DEV] i18n routing', () => { await devServer.stop(); }); - it('should render the en locale', async () => { - const response = await fixture.fetch('/new-site/en/start'); + it('should render the default locale without prefix', async () => { + const response = await fixture.fetch('/new-site/start'); expect(response.status).to.equal(200); - expect(await response.text()).includes('Hello'); + expect(await response.text()).includes('Start'); - const response2 = await fixture.fetch('/new-site/en/blog/1'); + const response2 = await fixture.fetch('/new-site/blog/1'); expect(response2.status).to.equal(200); expect(await response2.text()).includes('Hello world'); }); @@ -140,7 +140,7 @@ describe('[DEV] i18n routing', () => { it('should render localised page correctly', async () => { const response = await fixture.fetch('/new-site/pt/start'); expect(response.status).to.equal(200); - expect(await response.text()).includes('Hola'); + expect(await response.text()).includes('Oi essa e start'); const response2 = await fixture.fetch('/new-site/pt/blog/1'); expect(response2.status).to.equal(200); @@ -150,7 +150,7 @@ describe('[DEV] i18n routing', () => { it('should redirect to the english locale, which is the first fallback', async () => { const response = await fixture.fetch('/new-site/it/start'); expect(response.status).to.equal(200); - expect(await response.text()).includes('Hello'); + expect(await response.text()).includes('Start'); }); it("should render a 404 because the route `fr` isn't included in the list of locales of the configuration", async () => { @@ -281,7 +281,7 @@ describe('[SSG] i18n routing', () => { defaultLocale: 'en', locales: ['en', 'pt', 'it'], fallback: { - it: ['en'], + it: 'en', }, fallbackControl: 'redirect', }, @@ -291,11 +291,11 @@ describe('[SSG] i18n routing', () => { }); it('should render the en locale', async () => { - let html = await fixture.readFile('/en/start/index.html'); + let html = await fixture.readFile('/start/index.html'); let $ = cheerio.load(html); - expect($('body').text()).includes('Hello'); + expect($('body').text()).includes('Start'); - html = await fixture.readFile('/en/blog/1/index.html'); + html = await fixture.readFile('/blog/1/index.html'); $ = cheerio.load(html); expect($('body').text()).includes('Hello world'); }); @@ -303,7 +303,7 @@ describe('[SSG] i18n routing', () => { it('should render localised page correctly', async () => { let html = await fixture.readFile('/pt/start/index.html'); let $ = cheerio.load(html); - expect($('body').text()).includes('Hola'); + expect($('body').text()).includes('Oi essa e start'); html = await fixture.readFile('/pt/blog/1/index.html'); $ = cheerio.load(html); @@ -314,7 +314,7 @@ describe('[SSG] i18n routing', () => { const html = await fixture.readFile('/it/start/index.html'); expect(html).to.include('http-equiv="refresh'); console.log(html); - expect(html).to.include('url=/new-site/en/start'); + expect(html).to.include('url=/new-site/start'); }); it("should render a 404 because the route `fr` isn't included in the list of locales of the configuration", async () => { @@ -420,7 +420,7 @@ describe('[SSR] i18n routing', () => { before(async () => { fixture = await loadFixture({ - root: './fixtures/i18n-routing/', + root: './fixtures/i18n-routing-fallback/', output: 'server', adapter: testAdapter(), experimental: { @@ -428,7 +428,7 @@ describe('[SSR] i18n routing', () => { defaultLocale: 'en', locales: ['en', 'pt', 'it'], fallback: { - it: ['en'], + it: 'en', }, fallbackControl: 'redirect', }, @@ -439,24 +439,24 @@ describe('[SSR] i18n routing', () => { }); it('should render the en locale', async () => { - let request = new Request('http://example.com/new-site/en/start'); + let request = new Request('http://example.com/new-site/start'); let response = await app.render(request); expect(response.status).to.equal(200); - expect(await response.text()).includes('Hello'); + expect(await response.text()).includes('Start'); }); it('should render localised page correctly', async () => { let request = new Request('http://example.com/new-site/pt/start'); let response = await app.render(request); expect(response.status).to.equal(200); - expect(await response.text()).includes('Hola'); + expect(await response.text()).includes('Oi essa e start'); }); it('should redirect to the english locale, which is the first fallback', async () => { let request = new Request('http://example.com/new-site/it/start'); let response = await app.render(request); expect(response.status).to.equal(302); - expect(response.headers.get('location')).to.equal('/new-site/en/start'); + expect(response.headers.get('location')).to.equal('/new-site/start'); }); it("should render a 404 because the route `fr` isn't included in the list of locales of the configuration", async () => { diff --git a/packages/astro/test/units/config/config-validate.test.js b/packages/astro/test/units/config/config-validate.test.js index be5923a04b4a..9bf56a1f3e43 100644 --- a/packages/astro/test/units/config/config-validate.test.js +++ b/packages/astro/test/units/config/config-validate.test.js @@ -105,7 +105,7 @@ describe('Config Validation', () => { defaultLocale: 'en', locales: ['es', 'en'], fallback: { - es: ['it'], + es: 'it', }, }, }, @@ -126,7 +126,7 @@ describe('Config Validation', () => { defaultLocale: 'en', locales: ['es', 'en'], fallback: { - it: ['en'], + it: 'en', }, }, },