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.

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

refactor(i18n): simpler fallback system #8858

Merged
merged 1 commit into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ export type SSRManifest = {
};

export type SSRManifestI18n = {
fallback?: Record<string, string[]>;
fallback?: Record<string, string>;
fallbackControl?: 'none' | 'redirect' | 'render';
locales: string[];
defaultLocale: string;
Expand Down
20 changes: 9 additions & 11 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,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(),
Expand All @@ -318,21 +318,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.`,
});
}
}
}
Expand Down
68 changes: 31 additions & 37 deletions packages/astro/src/core/routing/manifest/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,46 +488,40 @@ export function createRouteManifest(
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}`)
for (const [fallbackFrom, fallbackTo] of fallback) {
const fallbackToRoutes = routes.filter((r) => r.component.includes(`/${fallbackTo}`));
const fallbackFromRoutes = routes.filter((r) => r.component.includes(`/${fallbackFrom}`));

for (const fallbackToRoute of fallbackToRoutes) {
const hasRoute = fallbackFromRoutes.find((r) => {
return (
r.component.replace(`/${fallbackTo}`, `/${fallbackFrom}`) ===
fallbackToRoute.component
);
});

if (!hasRoute) {
const pathname = fallbackToRoute.pathname?.replace(
`/${fallbackLocaleEntry}`,
`/${fallbackLocale}`
);
const route = fallbackToRoute.route?.replace(
`/${fallbackLocaleEntry}`,
`/${fallbackLocale}`
);

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',
if (!hasRoute) {
const pathname = fallbackToRoute.pathname?.replace(
`/${fallbackTo}`,
`/${fallbackFrom}`
);
const route = fallbackToRoute.route?.replace(`/${fallbackTo}`, `/${fallbackFrom}`);

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',
});
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions packages/astro/src/i18n/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ export function createI18nMiddleware(
const urlLocale = separators.find((s) => locales.includes(s));

if (urlLocale && fallbackKeys.includes(urlLocale)) {
// TODO: correctly handle chain of fallback
const fallbackLocale = i18n.fallback[urlLocale][0];
const fallbackLocale = i18n.fallback[urlLocale];
const newPathname = url.pathname.replace(`/${urlLocale}`, `/${fallbackLocale}`);
return context.redirect(newPathname);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export default defineConfig({
'en', 'pt', 'it'
],
fallback: {
"it": ["en"]
"it": "en"
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Continue
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
End
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Continuar pt
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Hola
</body>
</html>
50 changes: 42 additions & 8 deletions packages/astro/test/i18-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ describe('[DEV] i18n routing', () => {
experimental: {
i18n: {
defaultLocale: 'en',
locales: ['en', 'pt', 'it'],
locales: ['en', 'pt', 'pt_BR', 'it'],
fallback: {
it: ['en'],
it: 'en',
pt_BR: 'pt',
},
fallbackControl: 'redirect',
},
Expand All @@ -138,10 +139,14 @@ describe('[DEV] i18n routing', () => {
});

it('should render localised page correctly', async () => {
const response = await fixture.fetch('/new-site/pt/start');
let response = await fixture.fetch('/new-site/pt/start');
expect(response.status).to.equal(200);
expect(await response.text()).includes('Hola');

response = await fixture.fetch('/new-site/pt/continue');
expect(response.status).to.equal(200);
expect(await response.text()).includes('Continuar pt');

const response2 = await fixture.fetch('/new-site/pt/blog/1');
expect(response2.status).to.equal(200);
expect(await response2.text()).includes('Hola mundo');
Expand All @@ -157,6 +162,17 @@ describe('[DEV] i18n routing', () => {
const response = await fixture.fetch('/new-site/fr/start');
expect(response.status).to.equal(404);
});

it('pt_BR should redirect to pt because the route exists in the first fallback', async () => {
const response = await fixture.fetch('/new-site/pt_BR/continue');
expect(response.status).to.equal(200);
expect(await response.text()).includes('Continuar pt');
});

it('pt_BR should render a 404 because the pt page is missing', async () => {
const response = await fixture.fetch('/new-site/pt_BR/end');
expect(response.status).to.equal(404);
});
});
});

Expand Down Expand Up @@ -279,9 +295,10 @@ describe('[SSG] i18n routing', () => {
experimental: {
i18n: {
defaultLocale: 'en',
locales: ['en', 'pt', 'it'],
locales: ['en', 'pt', 'pt_BR', 'it'],
fallback: {
it: ['en'],
it: 'en',
pt_BR: 'pt',
},
fallbackControl: 'redirect',
},
Expand Down Expand Up @@ -313,7 +330,6 @@ describe('[SSG] i18n routing', () => {
it('should redirect to the english locale, which is the first fallback', async () => {
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');
});

Expand All @@ -327,6 +343,23 @@ describe('[SSG] i18n routing', () => {
return true;
}
});

it('pt_BR should redirect to pt because it is in the fallback', async () => {
const html = await fixture.readFile('/pt_BR/continue/index.html');
expect(html).to.include('http-equiv="refresh');
expect(html).to.include('url=/new-site/pt/continue');
});

it('pt_BR should render a 404 because the pt page is missing', async () => {
try {
await fixture.readFile('/pt_BR/end/index.html');
// failed
return false;
} catch {
// success
return true;
}
});
});
});

Expand Down Expand Up @@ -426,9 +459,10 @@ describe('[SSR] i18n routing', () => {
experimental: {
i18n: {
defaultLocale: 'en',
locales: ['en', 'pt', 'it'],
locales: ['en', 'pt', 'pt_BR', 'it'],
fallback: {
it: ['en'],
it: 'en',
pt_BR: 'pt',
},
fallbackControl: 'redirect',
},
Expand Down
4 changes: 2 additions & 2 deletions packages/astro/test/units/config/config-validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ describe('Config Validation', () => {
defaultLocale: 'en',
locales: ['es', 'en'],
fallback: {
es: ['it'],
es: 'it',
},
},
},
Expand All @@ -126,7 +126,7 @@ describe('Config Validation', () => {
defaultLocale: 'en',
locales: ['es', 'en'],
fallback: {
it: ['en'],
it: 'en',
},
},
},
Expand Down
18 changes: 18 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading