Skip to content

Commit

Permalink
fix(i18n): regression where emitted URLs had // (#10067)
Browse files Browse the repository at this point in the history
* fix(i18n): regression

* fix(i18n): regression

* fix bug regression

* remove double changeset
  • Loading branch information
ematipico committed Feb 9, 2024
1 parent 79e832f commit 989ea63
Show file tree
Hide file tree
Showing 8 changed files with 157 additions and 147 deletions.
5 changes: 5 additions & 0 deletions .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.
2 changes: 1 addition & 1 deletion packages/astro/src/core/app/index.ts
Expand Up @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/plugins/plugin-manifest.ts
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions packages/astro/src/core/config/schema.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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';
Expand Down Expand Up @@ -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({
Expand Down Expand Up @@ -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) {
Expand Down
100 changes: 18 additions & 82 deletions packages/astro/src/i18n/index.ts
Expand Up @@ -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);
Expand All @@ -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 {
Expand Down Expand Up @@ -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 });
});
}

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

/**
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/i18n/middleware.ts
Expand Up @@ -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) {
Expand Down
15 changes: 3 additions & 12 deletions packages/astro/test/i18n-routing.nodetest.js
Expand Up @@ -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();
Expand All @@ -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);
});
});
});
Expand Down

0 comments on commit 989ea63

Please sign in to comment.