From 50840fae8aa96dd0c59958b443fda039eae098db Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 9 Nov 2023 13:14:50 +0000 Subject: [PATCH 1/2] fix(i18n): fallback index when routing is prefix-always --- .../astro/src/core/build/buildPipeline.ts | 6 +- .../astro/src/core/routing/manifest/create.ts | 158 +++++++++++------- .../src/vite-plugin-astro-server/route.ts | 2 +- .../src/pages/index.astro | 8 - 4 files changed, 105 insertions(+), 69 deletions(-) delete mode 100644 packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/index.astro diff --git a/packages/astro/src/core/build/buildPipeline.ts b/packages/astro/src/core/build/buildPipeline.ts index b668c0756579..83e45f808371 100644 --- a/packages/astro/src/core/build/buildPipeline.ts +++ b/packages/astro/src/core/build/buildPipeline.ts @@ -161,7 +161,11 @@ export class BuildPipeline extends Pipeline { for (const pageData of pageDataList) { if (routeIsRedirect(pageData.route)) { pages.set(pageData, path); - } else if (routeIsFallback(pageData.route) && i18nHasFallback(this.getConfig())) { + } else if ( + routeIsFallback(pageData.route) && + (i18nHasFallback(this.getConfig()) || + (routeIsFallback(pageData.route) && pageData.route.route === '/')) + ) { pages.set(pageData, path); } } diff --git a/packages/astro/src/core/routing/manifest/create.ts b/packages/astro/src/core/routing/manifest/create.ts index 6f2409091286..f5b6e0ccdda3 100644 --- a/packages/astro/src/core/routing/manifest/create.ts +++ b/packages/astro/src/core/routing/manifest/create.ts @@ -485,13 +485,13 @@ export function createRouteManifest( routes.push(routeData); }); const i18n = settings.config.experimental.i18n; - - if (i18n && i18n.fallback) { - let fallback = Object.entries(i18n.fallback); + if (i18n) { + // In this block of code we group routes based on their locale // 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 + // This type is here only as a helper. We copy the routes and make them unique, so we don't "process" the same route twice. + // The assumption is that a route in the file system belongs to only one locale. const setRoutes = new Set(routes); // First loop @@ -524,70 +524,110 @@ export function createRouteManifest( setRoutes.delete(route); } - if (fallback.length > 0) { - 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); + // Work done, now we start creating "fallback" routes based on the configuration - // Technically, we should always have a fallback to. Added this to make TS happy. - if (!fallbackToRoutes) { - continue; + if (i18n.routingStrategy === 'prefix-always') { + // we attempt to retrieve the index page of the default locale + const defaultLocaleRoutes = routesByLocale.get(i18n.defaultLocale); + if (defaultLocaleRoutes) { + const indexDefaultRoute = defaultLocaleRoutes.find((routeData) => { + // it should be safe to assume that an index page has "index" in their name + return routeData.component.includes('index'); + }); + if (indexDefaultRoute) { + // we found the index of the default locale, now we create a root index that will redirect to the index of the default locale + const pathname = '/'; + const route = '/'; + + const segments = removeLeadingForwardSlash(route) + .split(path.posix.sep) + .filter(Boolean) + .map((s: string) => { + validateSegment(s); + return getParts(s, route); + }); + routes.push({ + ...indexDefaultRoute, + pathname, + route, + segments, + pattern: getPattern(segments, config), + type: 'fallback', + }); } + } + } + + if (i18n.fallback) { + let fallback = Object.entries(i18n.fallback); + + if (fallback.length > 0) { + 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); - for (const fallbackToRoute of fallbackToRoutes) { - const hasRoute = - fallbackFromRoutes && - // we check if the fallback from locale (the origin) has already this route - fallbackFromRoutes.some((route) => { + // 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) { - return route.route.replace(`/${fallbackFromLocale}`, '') === fallbackToRoute.route; + if (fallbackToRoute.pathname) { + pathname = `/${fallbackFromLocale}${fallbackToRoute.pathname}`; + } + route = `/${fallbackFromLocale}${fallbackToRoute.route}`; } else { - return ( - route.route.replace(`/${fallbackToLocale}`, `/${fallbackFromLocale}`) === - fallbackToRoute.route + pathname = fallbackToRoute.pathname?.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` + ); + route = fallbackToRoute.route.replace( + `/${fallbackToLocale}`, + `/${fallbackFromLocale}` ); } - }); - - 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}` - ); - 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); + 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', }); - routes.push({ - ...fallbackToRoute, - pathname, - route, - segments, - pattern: getPattern(segments, config), - type: 'fallback', - }); + } } } } diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index 2b0bcbea75ae..a54ece17836f 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -187,7 +187,7 @@ export async function handleRoute({ .some((segment) => { return locales.includes(segment); }); - if (!pathNameHasLocale) { + if (!pathNameHasLocale && pathname !== '/') { return handle404Response(origin, incomingRequest, incomingResponse); } request = createRequest({ diff --git a/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/index.astro b/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/index.astro deleted file mode 100644 index 05faf7b0bcce..000000000000 --- a/packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/index.astro +++ /dev/null @@ -1,8 +0,0 @@ - - - Astro - - - Hello - - From dcacacfc5a66883611daf758711f484714bba566 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Thu, 9 Nov 2023 14:11:03 +0000 Subject: [PATCH 2/2] chore: add comment as per feedback --- packages/astro/src/vite-plugin-astro-server/route.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/astro/src/vite-plugin-astro-server/route.ts b/packages/astro/src/vite-plugin-astro-server/route.ts index a54ece17836f..89173a1ecf8d 100644 --- a/packages/astro/src/vite-plugin-astro-server/route.ts +++ b/packages/astro/src/vite-plugin-astro-server/route.ts @@ -187,6 +187,7 @@ export async function handleRoute({ .some((segment) => { return locales.includes(segment); }); + // Even when we have `config.base`, the pathname is still `/` because it gets stripped before if (!pathNameHasLocale && pathname !== '/') { return handle404Response(origin, incomingRequest, incomingResponse); }