Skip to content

Commit

Permalink
Fix language switcher trailingSlash edge case (#2616)
Browse files Browse the repository at this point in the history
  • Loading branch information
delucis authored Nov 19, 2024
1 parent 0889189 commit 128cc51
Show file tree
Hide file tree
Showing 8 changed files with 83 additions and 33 deletions.
5 changes: 5 additions & 0 deletions .changeset/thick-cooks-call.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@astrojs/starlight': patch
---

Fixes an edge case to correctly avoid a trailing slash when navigating from a root locale homepage to another language via Starlight’s language switcher when `trailingSlash: 'never'` is set
8 changes: 4 additions & 4 deletions packages/starlight/__tests__/basics/localizedUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,23 @@ import { localizedUrl } from '../../utils/localizedUrl';
describe('with `build.output: "directory"`', () => {
test('it has no effect in a monolingual project', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, undefined).href).toBe(url.href);
expect(localizedUrl(url, undefined, 'ignore').href).toBe(url.href);
});

test('has no effect on index route in a monolingual project', () => {
const url = new URL('https://example.com/');
expect(localizedUrl(url, undefined).href).toBe(url.href);
expect(localizedUrl(url, undefined, 'ignore').href).toBe(url.href);
});
});

describe('with `build.output: "file"`', () => {
test('it has no effect in a monolingual project', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, undefined).href).toBe(url.href);
expect(localizedUrl(url, undefined, 'ignore').href).toBe(url.href);
});

test('has no effect on index route in a monolingual project', () => {
const url = new URL('https://example.com/index.html');
expect(localizedUrl(url, undefined).href).toBe(url.href);
expect(localizedUrl(url, undefined, 'ignore').href).toBe(url.href);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,23 @@ import { localizedUrl } from '../../utils/localizedUrl';
describe('with `build.output: "directory"`', () => {
test('it has no effect in a monolingual project with a non-root single locale', () => {
const url = new URL('https://example.com/fr/guide/');
expect(localizedUrl(url, 'fr').href).toBe(url.href);
expect(localizedUrl(url, 'fr', 'ignore').href).toBe(url.href);
});

test('has no effect on index route in a monolingual project with a non-root single locale', () => {
const url = new URL('https://example.com/fr/');
expect(localizedUrl(url, 'fr').href).toBe(url.href);
expect(localizedUrl(url, 'fr', 'ignore').href).toBe(url.href);
});
});

describe('with `build.output: "file"`', () => {
test('it has no effect in a monolingual project with a non-root single locale', () => {
const url = new URL('https://example.com/fr/guide.html');
expect(localizedUrl(url, 'fr').href).toBe(url.href);
expect(localizedUrl(url, 'fr', 'ignore').href).toBe(url.href);
});

test('has no effect on index route in a monolingual project with a non-root single locale', () => {
const url = new URL('https://example.com/fr.html');
expect(localizedUrl(url, 'fr').href).toBe(url.href);
expect(localizedUrl(url, 'fr', 'ignore').href).toBe(url.href);
});
});
65 changes: 51 additions & 14 deletions packages/starlight/__tests__/i18n-root-locale/localizedUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,73 +4,110 @@ import { localizedUrl } from '../../utils/localizedUrl';
describe('with `build.output: "directory"`', () => {
test('it has no effect if locale matches', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it has no effect if locale matches for index', () => {
const url = new URL('https://example.com/en/');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it changes locale to requested locale', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, 'ar').href).toBe('https://example.com/ar/guide/');
expect(localizedUrl(url, 'ar', 'ignore').href).toBe('https://example.com/ar/guide/');
});

test('it changes locale to requested locale for index', () => {
const url = new URL('https://example.com/en/');
expect(localizedUrl(url, 'ar').href).toBe('https://example.com/ar/');
expect(localizedUrl(url, 'ar', 'ignore').href).toBe('https://example.com/ar/');
});

test('it can change to root locale', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, undefined).href).toBe('https://example.com/guide/');
expect(localizedUrl(url, undefined, 'ignore').href).toBe('https://example.com/guide/');
});

test('it can change from root locale', () => {
const url = new URL('https://example.com/guide/');
expect(localizedUrl(url, 'en').href).toBe('https://example.com/en/guide/');
expect(localizedUrl(url, 'en', 'ignore').href).toBe('https://example.com/en/guide/');
});
});

describe('with `trailingSlash: "never"`', () => {
test('it has no effect if locale matches', () => {
const url = new URL('https://example.com/en/guide');
expect(localizedUrl(url, 'en', 'never').href).toBe(url.href);
});

test('it has no effect if locale matches for index', () => {
const url = new URL('https://example.com/en');
expect(localizedUrl(url, 'en', 'never').href).toBe(url.href);
});

test('it changes locale to requested locale', () => {
const url = new URL('https://example.com/en/guide');
expect(localizedUrl(url, 'ar', 'never').href).toBe('https://example.com/ar/guide');
});

test('it changes locale to requested locale for index', () => {
const url = new URL('https://example.com/en');
expect(localizedUrl(url, 'ar', 'never').href).toBe('https://example.com/ar');
});

test('it can change to root locale', () => {
const url = new URL('https://example.com/en/guide');
expect(localizedUrl(url, undefined, 'ignore').href).toBe('https://example.com/guide');
});

test('it can change from root locale', () => {
const url = new URL('https://example.com/guide');
expect(localizedUrl(url, 'en', 'ignore').href).toBe('https://example.com/en/guide');
});

test('it can change from root index', () => {
const url = new URL('https://example.com');
expect(localizedUrl(url, 'en', 'never').href).toBe('https://example.com/en');
});
});

describe('with `build.output: "file"`', () => {
test('it has no effect if locale matches', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it has no effect if locale matches for index', () => {
const url = new URL('https://example.com/en.html');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it changes locale to requested locale', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, 'ar').href).toBe('https://example.com/ar/guide.html');
expect(localizedUrl(url, 'ar', 'ignore').href).toBe('https://example.com/ar/guide.html');
});

test('it changes locale to requested locale for index', () => {
const url = new URL('https://example.com/en.html');
expect(localizedUrl(url, 'ar').href).toBe('https://example.com/ar.html');
expect(localizedUrl(url, 'ar', 'ignore').href).toBe('https://example.com/ar.html');
});

test('it can change to root locale', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, undefined).href).toBe('https://example.com/guide.html');
expect(localizedUrl(url, undefined, 'ignore').href).toBe('https://example.com/guide.html');
});

test('it can change to root locale from index', () => {
const url = new URL('https://example.com/en.html');
expect(localizedUrl(url, undefined).href).toBe('https://example.com/index.html');
expect(localizedUrl(url, undefined, 'ignore').href).toBe('https://example.com/index.html');
});

test('it can change from root locale', () => {
const url = new URL('https://example.com/guide.html');
expect(localizedUrl(url, 'en').href).toBe('https://example.com/en/guide.html');
expect(localizedUrl(url, 'en', 'ignore').href).toBe('https://example.com/en/guide.html');
});

test('it can change from root locale from index', () => {
const url = new URL('https://example.com/index.html');
expect(localizedUrl(url, 'en').href).toBe('https://example.com/en.html');
expect(localizedUrl(url, 'en', 'ignore').href).toBe('https://example.com/en.html');
});
});
16 changes: 8 additions & 8 deletions packages/starlight/__tests__/i18n/localizedUrl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,43 +4,43 @@ import { localizedUrl } from '../../utils/localizedUrl';
describe('with `build.output: "directory"`', () => {
test('it has no effect if locale matches', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it has no effect if locale matches for index', () => {
const url = new URL('https://example.com/en/');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it changes locale to requested locale', () => {
const url = new URL('https://example.com/en/guide/');
expect(localizedUrl(url, 'fr').href).toBe('https://example.com/fr/guide/');
expect(localizedUrl(url, 'fr', 'ignore').href).toBe('https://example.com/fr/guide/');
});

test('it changes locale to requested locale for index', () => {
const url = new URL('https://example.com/en/');
expect(localizedUrl(url, 'fr').href).toBe('https://example.com/fr/');
expect(localizedUrl(url, 'fr', 'ignore').href).toBe('https://example.com/fr/');
});
});

describe('with `build.output: "file"`', () => {
test('it has no effect if locale matches', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it has no effect if locale matches for index', () => {
const url = new URL('https://example.com/en.html');
expect(localizedUrl(url, 'en').href).toBe(url.href);
expect(localizedUrl(url, 'en', 'ignore').href).toBe(url.href);
});

test('it changes locale to requested locale', () => {
const url = new URL('https://example.com/en/guide.html');
expect(localizedUrl(url, 'fr').href).toBe('https://example.com/fr/guide.html');
expect(localizedUrl(url, 'fr', 'ignore').href).toBe('https://example.com/fr/guide.html');
});

test('it changes locale to requested locale for index', () => {
const url = new URL('https://example.com/en.html');
expect(localizedUrl(url, 'fr').href).toBe('https://example.com/fr.html');
expect(localizedUrl(url, 'fr', 'ignore').href).toBe('https://example.com/fr.html');
});
});
3 changes: 2 additions & 1 deletion packages/starlight/components/Head.astro
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
---
import type { z } from 'astro/zod';
import context from 'virtual:starlight/project-context';
import config from 'virtual:starlight/user-config';
import { version } from '../package.json';
import type { HeadConfigSchema } from '../schemas/head';
Expand Down Expand Up @@ -66,7 +67,7 @@ if (canonical && config.isMultilingual) {
attrs: {
rel: 'alternate',
hreflang: localeOpts.lang,
href: localizedUrl(canonical, locale).href,
href: localizedUrl(canonical, locale, context.trailingSlash).href,
},
});
}
Expand Down
3 changes: 2 additions & 1 deletion packages/starlight/components/LanguageSelect.astro
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
---
import context from 'virtual:starlight/project-context';
import config from 'virtual:starlight/user-config';
import { localizedUrl } from '../utils/localizedUrl';
import Select from './Select.astro';
Expand All @@ -8,7 +9,7 @@ import type { Props } from '../props';
* Get the equivalent of the current page path for the passed locale.
*/
function localizedPathname(locale: string | undefined): string {
return localizedUrl(Astro.url, locale).pathname;
return localizedUrl(Astro.url, locale, context.trailingSlash).pathname;
}
---

Expand Down
8 changes: 7 additions & 1 deletion packages/starlight/utils/localizedUrl.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,15 @@
import config from 'virtual:starlight/user-config';
import { stripTrailingSlash } from './path';
import type { AstroConfig } from 'astro';

/**
* Get the equivalent of the passed URL for the passed locale.
*/
export function localizedUrl(url: URL, locale: string | undefined): URL {
export function localizedUrl(
url: URL,
locale: string | undefined,
trailingSlash: AstroConfig['trailingSlash']
): URL {
// Create a new URL object to void mutating the global.
url = new URL(url);
if (!config.locales) {
Expand Down Expand Up @@ -41,5 +46,6 @@ export function localizedUrl(url: URL, locale: string | undefined): URL {
}
// Restore base
if (hasBase) url.pathname = base + url.pathname;
if (trailingSlash === 'never') url.pathname = stripTrailingSlash(url.pathname);
return url;
}

0 comments on commit 128cc51

Please sign in to comment.