Skip to content

Commit 1e6017f

Browse files
ematipicomatthewp
andauthored
Refactor/unit tests part2 (#15990)
* fix(i18n): two locale and rendering bugs * fix regression * address feedback * Update .changeset/kind-pants-wish.md Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com> --------- Co-authored-by: Matthew Phillips <matthewphillips@cloudflare.com>
1 parent 4741b09 commit 1e6017f

File tree

11 files changed

+834
-10
lines changed

11 files changed

+834
-10
lines changed

.changeset/kind-pants-wish.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes an issue where `Astro.currentLocale` would always be the default locale instead of the actual one when using a dynamic route like `[locale].astro` or `[locale]/index.astro`. It now resolves to the correct locale from the URL.

.changeset/little-heads-kick.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'astro': patch
3+
---
4+
5+
Fixes an issue where visiting an invalid locale URL (e.g. `/asdf/`) would show the content of a dynamic `[locale]` page with a 404 status code, instead of showing your custom 404 page. Now, the correct 404 page is rendered when the locale in the URL doesn't match any configured locale.

packages/astro/src/core/render-context.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { ActionAPIContext } from '../actions/runtime/types.js';
55
import { createCallAction, createGetActionResult, hasActionPayload } from '../actions/utils.js';
66
import {
77
computeCurrentLocale,
8+
computeCurrentLocaleFromParams,
89
computePreferredLocale,
910
computePreferredLocaleList,
1011
} from '../i18n/utils.js';
@@ -898,6 +899,16 @@ export class RenderContext {
898899
}
899900
pathname = pathname && !isRoute404or500(routeData) ? pathname : url.pathname;
900901
computedLocale = computeCurrentLocale(pathname, locales, defaultLocale);
902+
// If the route has dynamic params, check if any param value matches a
903+
// configured locale. This handles routes like [locale].astro where the
904+
// pathname contains unresolved placeholders and computeCurrentLocale
905+
// falls back to the default locale.
906+
if (routeData.params.length > 0) {
907+
const localeFromParams = computeCurrentLocaleFromParams(this.params, locales);
908+
if (localeFromParams) {
909+
computedLocale = localeFromParams;
910+
}
911+
}
901912
}
902913

903914
this.#currentLocale = computedLocale ?? fallbackTo;

packages/astro/src/core/routing/match.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,17 @@ import { isRoute404, isRoute500 } from './internal/route-errors.js';
66

77
/** Find matching route from pathname */
88
export function matchRoute(pathname: string, manifest: RoutesList): RouteData | undefined {
9+
// Error pages (404/500) take precedence over dynamic routes that might
10+
// capture the same path (e.g. [locale] matching /404). See #15098.
11+
if (isRoute404(pathname)) {
12+
const errorRoute = manifest.routes.find((route) => isRoute404(route.route));
13+
if (errorRoute) return errorRoute;
14+
}
15+
if (isRoute500(pathname)) {
16+
const errorRoute = manifest.routes.find((route) => isRoute500(route.route));
17+
if (errorRoute) return errorRoute;
18+
}
19+
920
return manifest.routes.find((route) => {
1021
return (
1122
route.pattern.test(pathname) ||

packages/astro/src/core/routing/rewrite.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
} from '../path.js';
1515
import { createRequest } from '../request.js';
1616
import { DEFAULT_404_ROUTE } from './internal/astro-designed-error-pages.js';
17+
import { isRoute404, isRoute500 } from './internal/route-errors.js';
1718

1819
type FindRouteToRewrite = {
1920
payload: RewritePayload;
@@ -99,6 +100,22 @@ export function findRouteToRewrite({
99100
}
100101

101102
const decodedPathname = decodeURI(pathname);
103+
104+
// Error pages (404/500) take precedence over dynamic routes that might
105+
// capture the same path (e.g. [locale] matching /404). See #15098.
106+
if (isRoute404(decodedPathname)) {
107+
const errorRoute = routes.find((route) => route.route === '/404');
108+
if (errorRoute) {
109+
return { routeData: errorRoute, newUrl, pathname: decodedPathname };
110+
}
111+
}
112+
if (isRoute500(decodedPathname)) {
113+
const errorRoute = routes.find((route) => route.route === '/500');
114+
if (errorRoute) {
115+
return { routeData: errorRoute, newUrl, pathname: decodedPathname };
116+
}
117+
}
118+
102119
let foundRoute;
103120
for (const route of routes) {
104121
if (route.pattern.test(decodedPathname)) {

packages/astro/src/i18n/middleware.ts

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -71,15 +71,28 @@ export function createI18nMiddleware(
7171
return context.redirect(location, routeDecision.status);
7272
}
7373
case 'notFound': {
74-
const notFoundRes = new Response(response.body, {
75-
status: 404,
76-
headers: response.headers,
77-
});
78-
notFoundRes.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
74+
if (context.isPrerendered) {
75+
// Prerendered pages are authored content — preserve the body so the
76+
// build pipeline can write the file. The REROUTE_DIRECTIVE prevents
77+
// the App from rerouting to the error page.
78+
const prerenderedRes = new Response(response.body, {
79+
status: 404,
80+
headers: response.headers,
81+
});
82+
prerenderedRes.headers.set(REROUTE_DIRECTIVE_HEADER, 'no');
83+
if (routeDecision.location) {
84+
prerenderedRes.headers.set('Location', routeDecision.location);
85+
}
86+
return prerenderedRes;
87+
}
88+
// For SSR, return a null-body 404 so the App reroutes to the actual
89+
// 404 page. This prevents dynamic routes like [locale] from serving
90+
// their content for invalid locale paths.
91+
const headers = new Headers();
7992
if (routeDecision.location) {
80-
notFoundRes.headers.set('Location', routeDecision.location);
93+
headers.set('Location', routeDecision.location);
8194
}
82-
return notFoundRes;
95+
return new Response(null, { status: 404, headers });
8396
}
8497
case 'continue':
8598
break; // Continue to fallback check

packages/astro/src/i18n/utils.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,3 +187,38 @@ export function computeCurrentLocale(
187187
}
188188
}
189189
}
190+
191+
/**
192+
* Check if any of the route's resolved param values match a configured locale.
193+
* This handles dynamic routes like `[locale]` or `[...path]` where the locale
194+
* isn't in a static segment of the route pathname.
195+
*/
196+
export function computeCurrentLocaleFromParams(
197+
params: Record<string, string | undefined>,
198+
locales: Locales,
199+
): string | undefined {
200+
// Precompute lookup maps for O(1) matching instead of nested loops.
201+
// normalizedCode -> original locale string or first code for object locales
202+
const byNormalizedCode = new Map<string, string>();
203+
// path -> first code (for object locales)
204+
const byPath = new Map<string, string>();
205+
206+
for (const locale of locales) {
207+
if (typeof locale === 'string') {
208+
byNormalizedCode.set(normalizeTheLocale(locale), locale);
209+
} else {
210+
byPath.set(locale.path, locale.codes[0]);
211+
for (const code of locale.codes) {
212+
byNormalizedCode.set(normalizeTheLocale(code), code);
213+
}
214+
}
215+
}
216+
217+
for (const value of Object.values(params)) {
218+
if (!value) continue;
219+
const pathMatch = byPath.get(value);
220+
if (pathMatch) return pathMatch;
221+
const codeMatch = byNormalizedCode.get(normalizeTheLocale(value));
222+
if (codeMatch) return codeMatch;
223+
}
224+
}

packages/astro/test/units/app/test-helpers.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
* @param {Function} [options.middleware]
1010
* @param {Function} [options.actions]
1111
* @param {number} [options.actionBodySizeLimit]
12+
* @param {object} [options.i18n]
1213
*/
1314
export function createManifest({
1415
routes,
@@ -18,6 +19,7 @@ export function createManifest({
1819
middleware = undefined,
1920
actions = undefined,
2021
actionBodySizeLimit = 0,
22+
i18n = undefined,
2123
} = {}) {
2224
const rootDir = new URL('file:///astro-test/');
2325
const buildDir = new URL('file:///astro-test/dist/');
@@ -44,7 +46,7 @@ export function createManifest({
4446
pageMap,
4547
serverIslandMappings: undefined,
4648
key: Promise.resolve(/** @type {CryptoKey} */ ({})),
47-
i18n: undefined,
49+
i18n,
4850
middleware,
4951
actions,
5052
sessionDriver: undefined,

0 commit comments

Comments
 (0)