Skip to content

Commit

Permalink
refactor: simpler fallback system
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Oct 18, 2023
1 parent 5600b00 commit 2ff664d
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 62 deletions.
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.

0 comments on commit 2ff664d

Please sign in to comment.