Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(i18n): fallback index when routing is prefix-always #9032

Merged
merged 2 commits into from
Nov 9, 2023
Merged

fix(i18n): fallback index when routing is prefix-always #9032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50840fa
fix(i18n): fallback index when routing is prefix-always
ematipico Nov 9, 2023
dcacacf
chore: add comment as per feedback
ematipico Nov 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/astro/src/core/build/buildPipeline.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
158 changes: 99 additions & 59 deletions packages/astro/src/core/routing/manifest/create.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, RouteData[]>();
// 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
Expand Down Expand Up @@ -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',
});
}
}
}
Comment on lines +529 to +559
Copy link
Member Author

@ematipico ematipico Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the additional code. The rest was just moved to be like this:

if (i18n) {
	// new block
	if (i18n.routingStrategy === 'prefix-always') {}

	if (i18n.falklback) {}
}


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',
});
}
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ export async function handleRoute({
.some((segment) => {
return locales.includes(segment);
});
if (!pathNameHasLocale) {
// Even when we have `config.base`, the pathname is still `/` because it gets stripped before
if (!pathNameHasLocale && pathname !== '/') {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
return handle404Response(origin, incomingRequest, incomingResponse);
}
request = createRequest({
Expand Down
8 changes: 0 additions & 8 deletions packages/astro/test/fixtures/i18n-routing-prefix-always/src/pages/index.astro
View file
Open in desktop

This file was deleted.

Loading