Skip to content

Commit

Permalink
Fix invalid Astro i18n config with a single root locale (#1993)
Browse files Browse the repository at this point in the history
Co-authored-by: Chris Swithinbank <swithinbank@gmail.com>
  • Loading branch information
HiDeoo and delucis committed Jun 12, 2024
1 parent b765fb1 commit 60165b2
Show file tree
Hide file tree
Showing 13 changed files with 226 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/plenty-houses-heal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/starlight': patch
---

Fixes an i18n configuration issue when using a single root locale.
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import config from 'virtual:starlight/user-config';
import { expect, test } from 'vitest';

test('test suite is using correct env', () => {
expect(config.title).toMatchObject({ 'fr-CA': 'i18n with a single root locale' });
});

test('config.isMultilingual is false with a single root locale', () => {
expect(config.isMultilingual).toBe(false);
expect(config.locales).toBeUndefined();
});

test('config.defaultLocale is populated from the single root locale', () => {
expect(config.defaultLocale.lang).toBe('fr-CA');
expect(config.defaultLocale.dir).toBe('ltr');
expect(config.defaultLocale.locale).toBeUndefined();
});
17 changes: 17 additions & 0 deletions packages/starlight/__tests__/i18n-single-root-locale/i18n.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { assert, describe, expect, test } from 'vitest';
import config from 'virtual:starlight/user-config';
import { processI18nConfig } from '../../utils/i18n';

describe('processI18nConfig', () => {
test('returns the Astro i18n config for a monolingual site with a single root locale', () => {
const { astroI18nConfig, starlightConfig } = processI18nConfig(config, undefined);

expect(astroI18nConfig.defaultLocale).toBe('fr-CA');
expect(astroI18nConfig.locales).toEqual(['fr-CA']);
assert(typeof astroI18nConfig.routing !== 'string');
expect(astroI18nConfig.routing?.prefixDefaultLocale).toBe(false);

// The Starlight configuration should not be modified.
expect(config).toStrictEqual(starlightConfig);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { expect, test, vi } from 'vitest';
import { generateRouteData } from '../../utils/route-data';
import { routes } from '../../utils/routing';

vi.mock('astro:content', async () =>
(await import('../test-utils')).mockedAstroContent({
docs: [['index.mdx', { title: 'Accueil' }]],
})
);

test('includes localized labels (fr)', () => {
const route = routes[0]!;
const data = generateRouteData({
props: { ...route, headings: [{ depth: 1, slug: 'heading-1', text: 'Heading 1' }] },
url: new URL('https://example.com'),
});
expect(data.labels).toBeDefined();
expect(data.labels['skipLink.label']).toBe('Aller au contenu');
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { expect, test, vi } from 'vitest';
import { routes } from '../../utils/routing';

vi.mock('astro:content', async () =>
(await import('../test-utils')).mockedAstroContent({
docs: [
['index.mdx', { title: 'Accueil' }],
['guides/authoring-content.md', { title: 'Authoring content' }],
// @ts-expect-error — Using a slug not present in Starlight docs site
['en/index.mdx', { title: 'Not the home page' }],
],
})
);

test('route slugs are normalized', () => {
const indexRoute = routes.find((route) => route.id.startsWith('index.md'));
expect(indexRoute?.slug).toBe('');
});

test('routes have locale data added', () => {
for (const route of routes) {
expect(route.lang).toBe('fr-CA');
expect(route.dir).toBe('ltr');
expect(route.locale).toBeUndefined();
}
});

test('does not mark any route as fallback routes', () => {
const fallbacks = routes.filter((route) => route.isFallback);
expect(fallbacks.length).toBe(0);
});
36 changes: 36 additions & 0 deletions packages/starlight/__tests__/i18n-single-root-locale/slugs.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { describe, expect, test } from 'vitest';
import { localeToLang, localizedId, localizedSlug, slugToLocaleData } from '../../utils/slugs';

describe('slugToLocaleData', () => {
test('returns an undefined locale for root locale slugs', () => {
expect(slugToLocaleData('test').locale).toBeUndefined();
expect(slugToLocaleData('dir/test').locale).toBeUndefined();
});
test('returns default locale "fr" lang', () => {
expect(slugToLocaleData('test').lang).toBe('fr-CA');
expect(slugToLocaleData('dir/test').lang).toBe('fr-CA');
});
test('returns default locale "ltr" dir', () => {
expect(slugToLocaleData('test').dir).toBe('ltr');
expect(slugToLocaleData('dir/test').dir).toBe('ltr');
});
});

describe('localeToLang', () => {
test('returns lang for default locale', () => {
expect(localeToLang(undefined)).toBe('fr-CA');
});
});

describe('localizedId', () => {
test('returns unchanged for default locale', () => {
expect(localizedId('test.md', undefined)).toBe('test.md');
});
});

describe('localizedSlug', () => {
test('returns unchanged for default locale', () => {
expect(localizedSlug('test', undefined)).toBe('test');
expect(localizedSlug('dir/test', undefined)).toBe('dir/test');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"page.editLink": "Changer cette page"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import { describe, expect, test } from 'vitest';
import { createTranslationSystemFromFs } from '../../utils/translations-fs';

describe('createTranslationSystemFromFs', () => {
test('creates a translation system that returns default strings', () => {
const useTranslations = createTranslationSystemFromFs(
{
locales: undefined,
defaultLocale: { label: 'Français', locale: 'fr', dir: 'ltr' },
},
// Using non-existent `_src/` to ignore custom files in this test fixture.
{ srcDir: new URL('./_src/', import.meta.url) }
);
const t = useTranslations('fr');
expect(t('page.editLink')).toMatchInlineSnapshot('"Modifier cette page"');
});

test('creates a translation system that uses custom strings', () => {
const useTranslations = createTranslationSystemFromFs(
{
locales: undefined,
defaultLocale: { label: 'Français', locale: 'fr', dir: 'ltr' },
},
// Using `src/` to load custom files in this test fixture.
{ srcDir: new URL('./src/', import.meta.url) }
);
const t = useTranslations('fr');
expect(t('page.editLink')).toMatchInlineSnapshot('"Changer cette page"');
});

test('returns translation for unknown language', () => {
const useTranslations = createTranslationSystemFromFs(
{
locales: undefined,
defaultLocale: { label: 'Français', locale: 'fr', dir: 'ltr' },
},
// Using `src/` to load custom files in this test fixture.
{ srcDir: new URL('./src/', import.meta.url) }
);
const t = useTranslations('ar');
expect(t('page.editLink')).toMatchInlineSnapshot('"Changer cette page"');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { describe, expect, test, vi } from 'vitest';
import translations from '../../translations';
import { useTranslations } from '../../utils/translations';

vi.mock('astro:content', async () =>
(await import('../test-utils')).mockedAstroContent({
i18n: [['fr-CA', { 'page.editLink': 'Modifier cette doc!' }]],
})
);

describe('useTranslations()', () => {
test('uses user-defined translations', () => {
const t = useTranslations('fr');
expect(t('page.editLink')).toBe('Modifier cette doc!');
expect(t('page.editLink')).not.toBe(translations.fr?.['page.editLink']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import { describe, expect, test } from 'vitest';
import translations from '../../translations';
import { useTranslations } from '../../utils/translations';

describe('built-in translations', () => {
test('includes French', () => {
expect(translations).toHaveProperty('fr');
});
});

describe('useTranslations()', () => {
test('works when no i18n collection is available', () => {
const t = useTranslations('fr');
expect(t).toBeTypeOf('function');
expect(t('page.editLink')).toBe(translations.fr?.['page.editLink']);
});

test('returns default locale for unknown language', () => {
const locale = 'xx';
expect(translations).not.toHaveProperty(locale);
const t = useTranslations(locale);
expect(t('page.editLink')).toBe(translations.fr?.['page.editLink']);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { defineVitestConfig } from '../test-config';

export default defineVitestConfig({
title: 'i18n with a single root locale',
locales: {
root: { label: 'Français', lang: 'fr-CA' },
},
});
2 changes: 1 addition & 1 deletion packages/starlight/utils/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function getAstroI18nConfig(config: StarlightConfig): NonNullable<AstroConfig['i
path: locale === 'root' ? localeConfig?.lang ?? BuiltInDefaultLocale.lang : locale,
};
})
: [BuiltInDefaultLocale.lang],
: [config.defaultLocale.lang],
routing: {
prefixDefaultLocale:
// Sites with multiple languages without a root locale.
Expand Down
6 changes: 5 additions & 1 deletion packages/starlight/utils/user-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,11 @@ export const StarlightConfigSchema = UserConfigSchema.strict().transform(
// Monolingual sites with only one non-root locale needs their configuration to be defined in
// `config.locales` so that slugs can be correctly generated by taking into consideration the
// base path at which a language is served which is the key of the `config.locales` object.
if (locales !== undefined && configuredLocales.length >= 1) {
if (
locales !== undefined &&
(configuredLocales.length > 1 ||
(configuredLocales.length === 1 && locales.root === undefined))
) {
// Make sure we can find the default locale and if not, help the user set it.
// We treat the root locale as the default if present and no explicit default is set.
const defaultLocaleConfig = locales[defaultLocale || 'root'];
Expand Down

0 comments on commit 60165b2

Please sign in to comment.