From 989ea63bb2a5a670021541198aa70b8dc7c4bd2f Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 9 Feb 2024 14:58:09 +0000 Subject: [PATCH] fix(i18n): regression where emitted URLs had `//` (#10067) * fix(i18n): regression * fix(i18n): regression * fix bug regression * remove double changeset --- .changeset/tasty-actors-dance.md | 5 + packages/astro/src/core/app/index.ts | 2 +- .../src/core/build/plugins/plugin-manifest.ts | 2 +- packages/astro/src/core/config/schema.ts | 8 +- packages/astro/src/i18n/index.ts | 100 ++--------- packages/astro/src/i18n/middleware.ts | 2 +- packages/astro/test/i18n-routing.nodetest.js | 15 +- .../astro/test/units/i18n/astro_i18n.test.js | 170 +++++++++++++----- 8 files changed, 157 insertions(+), 147 deletions(-) create mode 100644 .changeset/tasty-actors-dance.md diff --git a/.changeset/tasty-actors-dance.md b/.changeset/tasty-actors-dance.md new file mode 100644 index 000000000000..e9ade9965192 --- /dev/null +++ b/.changeset/tasty-actors-dance.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes a regression in the `astro:i18n` module, where the functions `getAbsoluteLocaleUrl` and `getAbsoluteLocaleUrlList` returned a URL with double slash with a certain combination of options. diff --git a/packages/astro/src/core/app/index.ts b/packages/astro/src/core/app/index.ts index 108b0f231a77..d3f287efd382 100644 --- a/packages/astro/src/core/app/index.ts +++ b/packages/astro/src/core/app/index.ts @@ -194,7 +194,7 @@ export class App { this.#manifest.i18n && (this.#manifest.i18n.routing === 'domains-prefix-always' || this.#manifest.i18n.routing === 'domains-prefix-other-locales' || - this.#manifest.i18n.routing === 'domains-prefix-other-no-redirect') + this.#manifest.i18n.routing === 'domains-prefix-always-no-redirect') ) { // https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Forwarded-Host let host = request.headers.get('X-Forwarded-Host'); diff --git a/packages/astro/src/core/build/plugins/plugin-manifest.ts b/packages/astro/src/core/build/plugins/plugin-manifest.ts index 6b5413583553..8b97651dbfbf 100644 --- a/packages/astro/src/core/build/plugins/plugin-manifest.ts +++ b/packages/astro/src/core/build/plugins/plugin-manifest.ts @@ -241,7 +241,7 @@ function buildManifest( i18n.domains && (i18n.routing === 'domains-prefix-always' || i18n.routing === 'domains-prefix-other-locales' || - i18n.routing === 'domains-prefix-other-no-redirect') + i18n.routing === 'domains-prefix-always-no-redirect') ) { for (const [locale, domainValue] of Object.entries(i18n.domains)) { domainLookupTable[domainValue] = normalizeTheLocale(locale); diff --git a/packages/astro/src/core/config/schema.ts b/packages/astro/src/core/config/schema.ts index 95f33eb419fe..e840f93a91cd 100644 --- a/packages/astro/src/core/config/schema.ts +++ b/packages/astro/src/core/config/schema.ts @@ -72,7 +72,7 @@ export type RoutingStrategies = | 'pathname-prefix-always-no-redirect' | 'domains-prefix-always' | 'domains-prefix-other-locales' - | 'domains-prefix-other-no-redirect'; + | 'domains-prefix-always-no-redirect'; export const AstroConfigSchema = z.object({ root: z @@ -383,7 +383,7 @@ export const AstroConfigSchema = z.object({ if (routing.redirectToDefaultLocale) { strategy = 'domains-prefix-always'; } else { - strategy = 'domains-prefix-other-no-redirect'; + strategy = 'domains-prefix-always-no-redirect'; } } else { strategy = 'domains-prefix-other-locales'; @@ -439,7 +439,7 @@ export const AstroConfigSchema = z.object({ if (entries.length > 0) { if ( routing !== 'domains-prefix-other-locales' && - routing !== 'domains-prefix-other-no-redirect' && + routing !== 'domains-prefix-always-no-redirect' && routing !== 'domains-prefix-always' ) { ctx.addIssue({ @@ -627,7 +627,7 @@ export function createRelativeSchema(cmd: string, fileProtocolRoot: string) { if (experimental.i18nDomains) { if ( i18n?.routing === 'domains-prefix-other-locales' || - i18n?.routing === 'domains-prefix-other-no-redirect' || + i18n?.routing === 'domains-prefix-always-no-redirect' || i18n?.routing === 'domains-prefix-always' ) { if (!site) { diff --git a/packages/astro/src/i18n/index.ts b/packages/astro/src/i18n/index.ts index 6a8834b541d5..4d66a56c3fc8 100644 --- a/packages/astro/src/i18n/index.ts +++ b/packages/astro/src/i18n/index.ts @@ -57,7 +57,12 @@ export function getLocaleRelativeUrl({ } const pathsToJoin = [base, prependWith]; const normalizedLocale = normalizeLocale ? normalizeTheLocale(codeToUse) : codeToUse; - if (routing === 'pathname-prefix-always' || routing === 'pathname-prefix-always-no-redirect') { + if ( + routing === 'pathname-prefix-always' || + routing === 'pathname-prefix-always-no-redirect' || + routing === 'domains-prefix-always' || + routing === 'domains-prefix-always-no-redirect' + ) { pathsToJoin.push(normalizedLocale); } else if (locale !== defaultLocale) { pathsToJoin.push(normalizedLocale); @@ -78,7 +83,7 @@ export function getLocaleAbsoluteUrl({ site, isBuild, ...rest }: GetLocaleAbsolu const localeUrl = getLocaleRelativeUrl(rest); const { domains, locale } = rest; let url; - if (isBuild && domains) { + if (isBuild && domains && domains[locale]) { const base = domains[locale]; url = joinPaths(base, localeUrl.replace(`/${rest.locale}`, '')); } else { @@ -108,89 +113,24 @@ interface GetLocalesRelativeUrlList extends GetLocaleOptions { } export function getLocaleRelativeUrlList({ - base, locales: _locales, - trailingSlash, - format, - path, - prependWith, - normalizeLocale = true, - routing = 'pathname-prefix-other-locales', - defaultLocale, + ...rest }: GetLocalesRelativeUrlList) { const locales = toPaths(_locales); return locales.map((locale) => { - const pathsToJoin = [base, prependWith]; - const normalizedLocale = normalizeLocale ? normalizeTheLocale(locale) : locale; - - if (routing === 'pathname-prefix-always' || routing === 'pathname-prefix-always-no-redirect') { - pathsToJoin.push(normalizedLocale); - } else if (locale !== defaultLocale) { - pathsToJoin.push(normalizedLocale); - } - pathsToJoin.push(path); - if (shouldAppendForwardSlash(trailingSlash, format)) { - return appendForwardSlash(joinPaths(...pathsToJoin)); - } else { - return joinPaths(...pathsToJoin); - } + return getLocaleRelativeUrl({ ...rest, locales, locale }); }); } interface GetLocalesAbsoluteUrlList extends GetLocalesRelativeUrlList { - site?: AstroConfig['site']; + site: AstroConfig['site']; isBuild: boolean; } -export function getLocaleAbsoluteUrlList({ - base, - locales: _locales, - trailingSlash, - format, - path, - prependWith, - normalizeLocale = true, - routing = 'pathname-prefix-other-locales', - defaultLocale, - isBuild, - domains, - site, -}: GetLocalesAbsoluteUrlList) { - const locales = toPaths(_locales); +export function getLocaleAbsoluteUrlList(params: GetLocalesAbsoluteUrlList) { + const locales = toCodes(params.locales); return locales.map((currentLocale) => { - const pathsToJoin = []; - const normalizedLocale = normalizeLocale ? normalizeTheLocale(currentLocale) : currentLocale; - const domainBase = domains ? domains[currentLocale] : undefined; - if (isBuild && domainBase) { - if (domainBase) { - pathsToJoin.push(domainBase); - } else { - pathsToJoin.push(site); - } - pathsToJoin.push(base); - pathsToJoin.push(prependWith); - } else { - if (site) { - pathsToJoin.push(site); - } - pathsToJoin.push(base); - pathsToJoin.push(prependWith); - if ( - routing === 'pathname-prefix-always' || - routing === 'pathname-prefix-always-no-redirect' - ) { - pathsToJoin.push(normalizedLocale); - } else if (currentLocale !== defaultLocale) { - pathsToJoin.push(normalizedLocale); - } - } - - pathsToJoin.push(path); - if (shouldAppendForwardSlash(trailingSlash, format)) { - return appendForwardSlash(joinPaths(...pathsToJoin)); - } else { - return joinPaths(...pathsToJoin); - } + return getLocaleAbsoluteUrl({ ...params, locale: currentLocale }); }); } @@ -253,17 +193,13 @@ export function normalizeTheLocale(locale: string): string { * @param locales */ export function toCodes(locales: Locales): string[] { - const codes: string[] = []; - for (const locale of locales) { - if (typeof locale === 'string') { - codes.push(locale); + return locales.map((loopLocale) => { + if (typeof loopLocale === 'string') { + return loopLocale; } else { - for (const code of locale.codes) { - codes.push(code); - } + return loopLocale.codes[0]; } - } - return codes; + }); } /** diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index 1c05a956be1d..73a43d471c25 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -137,7 +137,7 @@ export function createI18nMiddleware( break; } - case 'domains-prefix-other-no-redirect': { + case 'domains-prefix-always-no-redirect': { if (localeHasntDomain(i18n, currentLocale)) { const result = prefixAlwaysNoRedirect(url, response); if (result) { diff --git a/packages/astro/test/i18n-routing.nodetest.js b/packages/astro/test/i18n-routing.nodetest.js index 1ef99e838a52..f8477a14e29a 100644 --- a/packages/astro/test/i18n-routing.nodetest.js +++ b/packages/astro/test/i18n-routing.nodetest.js @@ -1607,15 +1607,6 @@ describe('[SSR] i18n routing', () => { root: './fixtures/i18n-routing/', output: 'server', adapter: testAdapter(), - i18n: { - defaultLocale: 'en', - locales: [ - { - path: 'english', - codes: ['en', 'en-AU', 'pt-BR', 'es-US'], - }, - ], - }, }); await fixture.build(); app = await fixture.loadTestAdapterApp(); @@ -1624,14 +1615,14 @@ describe('[SSR] i18n routing', () => { it('they should be still considered when parsing the Accept-Language header', async () => { let request = new Request('http://example.com/preferred-locale', { headers: { - 'Accept-Language': 'en-AU;q=0.1,pt-BR;q=0.9', + 'Accept-Language': 'en-AU;q=0.1,es;q=0.9', }, }); let response = await app.render(request); const text = await response.text(); assert.equal(response.status, 200); - assert.equal(text.includes('Locale: english'), true); - assert.equal(text.includes('Locale list: english'), true); + assert.equal(text.includes('Locale: spanish'), true); + assert.equal(text.includes('Locale list: spanish'), true); }); }); }); diff --git a/packages/astro/test/units/i18n/astro_i18n.test.js b/packages/astro/test/units/i18n/astro_i18n.test.js index e3364924af8f..1e0d0475c975 100644 --- a/packages/astro/test/units/i18n/astro_i18n.test.js +++ b/packages/astro/test/units/i18n/astro_i18n.test.js @@ -6,6 +6,7 @@ import { } from '../../../dist/i18n/index.js'; import { parseLocale } from '../../../dist/core/render/context.js'; import { expect } from 'chai'; +import { validateConfig } from '../../../dist/core/config/config.js'; describe('getLocaleRelativeUrl', () => { it('should correctly return the URL with the base', () => { @@ -657,23 +658,21 @@ describe('getLocaleAbsoluteUrl', () => { */ const config = { base: '/blog', - experimental: { - i18n: { - defaultLocale: 'en', - locales: [ - 'en', - 'en_US', - 'es', - { - path: 'italiano', - codes: ['it', 'it-VA'], - }, - ], - domains: { - es: 'https://es.example.com', + i18n: { + defaultLocale: 'en', + locales: [ + 'en', + 'en_US', + 'es', + { + path: 'italiano', + codes: ['it', 'it-VA'], }, - routingStrategy: 'prefix-other-locales', + ], + domains: { + es: 'https://es.example.com', }, + routingStrategy: 'prefix-other-locales', }, }; @@ -685,14 +684,14 @@ describe('getLocaleAbsoluteUrl', () => { trailingSlash: 'always', format: 'directory', site: 'https://example.com', - ...config.experimental.i18n, + ...config.i18n, }) ).to.eq('https://example.com/blog/'); expect( getLocaleAbsoluteUrl({ locale: 'es', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'directory', site: 'https://example.com', @@ -703,7 +702,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'es', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'directory', site: 'https://example.com', @@ -715,7 +714,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'en_US', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'directory', site: 'https://example.com', @@ -727,7 +726,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'en', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -737,7 +736,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'es', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -748,7 +747,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'en_US', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -758,7 +757,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'it-VA', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -769,7 +768,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'en_US', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -780,7 +779,7 @@ describe('getLocaleAbsoluteUrl', () => { getLocaleAbsoluteUrl({ locale: 'es', base: '/blog/', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -793,7 +792,7 @@ describe('getLocaleAbsoluteUrl', () => { locale: 'es', base: '/blog/', prependWith: 'some-name', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', @@ -808,14 +807,14 @@ describe('getLocaleAbsoluteUrl', () => { locale: 'en', base: '/blog/', prependWith: 'some-name', - ...config.experimental.i18n, + ...config.i18n, trailingSlash: 'always', format: 'file', site: 'https://example.com', path: 'first-post', isBuild: true, }) - ).to.eq('/blog/some-name/first-post/'); + ).to.eq('https://example.com/blog/some-name/first-post/'); }); }); describe('with [prefix-always]', () => { @@ -1448,13 +1447,17 @@ describe('getLocaleAbsoluteUrl', () => { }); describe('getLocaleAbsoluteUrlList', () => { - it('should retrieve the correct list of base URL with locales [format: directory, trailingSlash: never]', () => { + it('should retrieve the correct list of base URL with locales [format: directory, trailingSlash: never]', async () => { /** * * @type {import("../../../dist/@types").AstroUserConfig} */ - const config = { - experimental: { + const config = await validateConfig( + { + trailingSlash: 'never', + format: 'directory', + site: 'https://example.com', + base: '/blog', i18n: { defaultLocale: 'en', locales: [ @@ -1468,16 +1471,15 @@ describe('getLocaleAbsoluteUrlList', () => { ], }, }, - }; + process.cwd() + ); // directory format expect( getLocaleAbsoluteUrlList({ locale: 'en', - base: '/blog', - ...config.experimental.i18n, - trailingSlash: 'never', - format: 'directory', - site: 'https://example.com', + ...config, + ...config.i18n, + isBuild: true, }) ).to.have.members([ 'https://example.com/blog', @@ -1487,28 +1489,30 @@ describe('getLocaleAbsoluteUrlList', () => { ]); }); - it('should retrieve the correct list of base URL with locales [format: directory, trailingSlash: always]', () => { + it('should retrieve the correct list of base URL with locales [format: directory, trailingSlash: always]', async () => { /** * * @type {import("../../../dist/@types").AstroUserConfig} */ - const config = { - experimental: { + const config = await validateConfig( + { + trailingSlash: 'always', + format: 'directory', + base: '/blog/', + site: 'https://example.com', i18n: { defaultLocale: 'en', locales: ['en', 'en_US', 'es'], }, }, - }; + process.cwd() + ); // directory format expect( getLocaleAbsoluteUrlList({ locale: 'en', - base: '/blog/', - ...config.experimental.i18n, - trailingSlash: 'always', - format: 'directory', - site: 'https://example.com', + ...config, + ...config.i18n, }) ).to.have.members([ 'https://example.com/blog/', @@ -1517,6 +1521,80 @@ describe('getLocaleAbsoluteUrlList', () => { ]); }); + it('should retrieve the correct list of base URL with locales and path [format: directory, trailingSlash: always]', async () => { + /** + * + * @type {import("../../../dist/@types").AstroUserConfig} + */ + const config = await validateConfig( + { + format: 'directory', + site: 'https://example.com/', + trailingSlash: 'always', + i18n: { + defaultLocale: 'en', + locales: ['en', 'en_US', 'es'], + routing: { + prefixDefaultLocale: true, + }, + }, + }, + process.cwd() + ); + // directory format + expect( + getLocaleAbsoluteUrlList({ + locale: 'en', + path: 'download', + ...config, + ...config.i18n, + }) + ).to.have.members([ + 'https://example.com/en/download/', + 'https://example.com/en-us/download/', + 'https://example.com/es/download/', + ]); + }); + + it('should retrieve the correct list of base URL with locales and path [format: directory, trailingSlash: always, domains]', async () => { + /** + * + * @type {import("../../../dist/@types").AstroUserConfig} + */ + const config = await validateConfig( + { + format: 'directory', + site: 'https://example.com/', + trailingSlash: 'always', + i18n: { + defaultLocale: 'en', + locales: ['en', 'en_US', 'es'], + routing: { + prefixDefaultLocale: true, + }, + domains: { + es: 'https://es.example.com', + }, + }, + }, + process.cwd() + ); + // directory format + expect( + getLocaleAbsoluteUrlList({ + locale: 'en', + path: 'download', + ...config, + ...config.i18n, + isBuild: true, + }) + ).to.have.members([ + 'https://example.com/en/download/', + 'https://example.com/en-us/download/', + 'https://es.example.com/download/', + ]); + }); + it('should retrieve the correct list of base URL with locales [format: file, trailingSlash: always]', () => { /** *