Infrastructure/astro
0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-02-17 22:44:24 -05:00
Code Issues Activity

fix(i18n): regression where emitted URLs had // (#10067)

Browse source 
* fix(i18n): regression

* fix(i18n): regression

* fix bug regression

* remove double changeset
This commit is contained in:
Emanuele Stoppa 2024-02-09 14:58:09 +00:00 • committed by GitHub
parent 79e832f8e4
commit 989ea63bb2
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 157 additions and 147 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/tasty-actors-dance.md Normal file
View file

@ -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.

2
packages/astro/src/core/app/index.ts
View file

@ -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');

2
packages/astro/src/core/build/plugins/plugin-manifest.ts
View file

@ -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);

8
packages/astro/src/core/config/schema.ts
View file

@ -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) {

100
packages/astro/src/i18n/index.ts
View file

@ -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;
});
}
/**

2
packages/astro/src/i18n/middleware.ts
View file

@ -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) {

15
packages/astro/test/i18n-routing.nodetest.js
View file

@ -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);
});
});
});

170
packages/astro/test/units/i18n/astro_i18n.test.js
View file

@ -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]', () => {
/**
*