From 5ead7851ed9df4c811c755e4aa81ee05e5416bee Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 31 Jan 2025 14:21:02 +0000 Subject: [PATCH 1/2] fix(i18n): server island request --- .changeset/silent-worms-whisper.md | 5 +++ packages/astro/src/core/render-context.ts | 4 +- packages/astro/src/core/routing/match.ts | 39 +++++++++++++++++++ .../astro/src/core/server-islands/endpoint.ts | 1 + packages/astro/src/i18n/index.ts | 7 ---- packages/astro/src/i18n/middleware.ts | 8 +++- .../i18n-server-island/astro.config.mjs | 17 ++++++++ .../fixtures/i18n-server-island/package.json | 8 ++++ .../src/components/Island.astro | 1 + .../src/pages/en/island.astro | 5 +++ .../i18n-server-island/src/pages/index.astro | 8 ++++ packages/astro/test/i18n-routing.test.js | 27 +++++++++++++ pnpm-lock.yaml | 6 +++ 13 files changed, 126 insertions(+), 10 deletions(-) create mode 100644 .changeset/silent-worms-whisper.md create mode 100644 packages/astro/test/fixtures/i18n-server-island/astro.config.mjs create mode 100644 packages/astro/test/fixtures/i18n-server-island/package.json create mode 100644 packages/astro/test/fixtures/i18n-server-island/src/components/Island.astro create mode 100644 packages/astro/test/fixtures/i18n-server-island/src/pages/en/island.astro create mode 100644 packages/astro/test/fixtures/i18n-server-island/src/pages/index.astro diff --git a/.changeset/silent-worms-whisper.md b/.changeset/silent-worms-whisper.md new file mode 100644 index 000000000000..d4d460eae918 --- /dev/null +++ b/.changeset/silent-worms-whisper.md @@ -0,0 +1,5 @@ +--- +'astro': patch +--- + +Fixes a bug where the i18n middleware was blocking a server island request when the `prefixDefaultLocale` option is set to `true` diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index 7f173a1de56e..fb262ee657f4 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -29,7 +29,7 @@ import { callMiddleware } from './middleware/callMiddleware.js'; import { sequence } from './middleware/index.js'; import { renderRedirect } from './redirects/render.js'; import { type Pipeline, Slots, getParams, getProps } from './render/index.js'; -import { isRoute404or500 } from './routing/match.js'; +import { isRoute404or500, isRouteServerIsland } from './routing/match.js'; import { copyRequest, getOriginPathname, setOriginPathname } from './routing/rewrite.js'; import { SERVER_ISLAND_COMPONENT } from './server-islands/endpoint.js'; import { AstroSession } from './session.js'; @@ -596,7 +596,7 @@ export class RenderContext { } let computedLocale; - if (routeData.component === SERVER_ISLAND_COMPONENT) { + if (isRouteServerIsland(routeData)) { let referer = this.request.headers.get('referer'); if (referer) { if (URL.canParse(referer)) { diff --git a/packages/astro/src/core/routing/match.ts b/packages/astro/src/core/routing/match.ts index 85be22dfb088..e446f6fa45d0 100644 --- a/packages/astro/src/core/routing/match.ts +++ b/packages/astro/src/core/routing/match.ts @@ -1,5 +1,6 @@ import type { RoutesList } from '../../types/astro.js'; import type { RouteData } from '../../types/public/internal.js'; +import { SERVER_ISLAND_BASE_PREFIX, SERVER_ISLAND_COMPONENT } from '../server-islands/endpoint.js'; /** Find matching route from pathname */ export function matchRoute(pathname: string, manifest: RoutesList): RouteData | undefined { @@ -37,3 +38,41 @@ export function isRoute500(route: string) { export function isRoute404or500(route: RouteData): boolean { return isRoute404(route.route) || isRoute500(route.route); } + +/** + * Determines if a given route is associated with the server island component. + * + * @param {RouteData} route - The route data object to evaluate. + * @return {boolean} Returns true if the route's component is the server island component, otherwise false. + */ +export function isRouteServerIsland(route: RouteData): boolean { + return route.component === SERVER_ISLAND_COMPONENT; +} + +/** + * Determines whether the given `Request` is targeted to a "server island" based on its URL. + * + * @param {Request} request - The request object to be evaluated. + * @param {string} [base=''] - The base path provided via configuration. + * @return {boolean} - Returns `true` if the request is for a server island, otherwise `false`. + */ +export function isRequestServerIsland(request: Request, base = ''): boolean { + const url = new URL(request.url); + const pathname = url.pathname.slice(base.length); + + return pathname.startsWith(SERVER_ISLAND_BASE_PREFIX); +} + +/** + * Checks if the given request corresponds to a 404 or 500 route based on the specified base path. + * + * @param {Request} request - The HTTP request object to be checked. + * @param {string} [base=''] - The base path to trim from the request's URL before checking the route. Default is an empty string. + * @return {boolean} Returns true if the request matches a 404 or 500 route; otherwise, returns false. + */ +export function requestIs404Or500(request: Request, base = '') { + const url = new URL(request.url); + const pathname = url.pathname.slice(base.length); + + return isRoute404(pathname) || isRoute500(pathname); +} diff --git a/packages/astro/src/core/server-islands/endpoint.ts b/packages/astro/src/core/server-islands/endpoint.ts index e23ef7f3da55..6a640ee41df7 100644 --- a/packages/astro/src/core/server-islands/endpoint.ts +++ b/packages/astro/src/core/server-islands/endpoint.ts @@ -13,6 +13,7 @@ import { getPattern } from '../routing/manifest/pattern.js'; export const SERVER_ISLAND_ROUTE = '/_server-islands/[name]'; export const SERVER_ISLAND_COMPONENT = '_server-islands.astro'; +export const SERVER_ISLAND_BASE_PREFIX = '_server-islands'; type ConfigFields = Pick; diff --git a/packages/astro/src/i18n/index.ts b/packages/astro/src/i18n/index.ts index 24283bc0eee0..246189f86ce0 100644 --- a/packages/astro/src/i18n/index.ts +++ b/packages/astro/src/i18n/index.ts @@ -16,13 +16,6 @@ export function requestHasLocale(locales: Locales) { }; } -export function requestIs404Or500(request: Request, base = '') { - const url = new URL(request.url); - const pathname = url.pathname.slice(base.length); - - return isRoute404(pathname) || isRoute500(pathname); -} - // Checks if the pathname has any locale export function pathHasLocale(path: string, locales: Locales): boolean { const segments = path.split('/'); diff --git a/packages/astro/src/i18n/middleware.ts b/packages/astro/src/i18n/middleware.ts index 6ee87606cd01..0f98fd4e21da 100644 --- a/packages/astro/src/i18n/middleware.ts +++ b/packages/astro/src/i18n/middleware.ts @@ -9,8 +9,8 @@ import { redirectToDefaultLocale, redirectToFallback, requestHasLocale, - requestIs404Or500, } from './index.js'; +import { isRequestServerIsland, requestIs404Or500 } from '../core/routing/match.js'; export function createI18nMiddleware( i18n: SSRManifest['i18n'], @@ -82,6 +82,12 @@ export function createI18nMiddleware( return response; } + // This is a case where the rendering phase belongs to a server island. Server island are + // special routes, and should be exhempt from i18n routing + if (isRequestServerIsland(context.request, base)) { + return response; + } + const { currentLocale } = context; switch (i18n.strategy) { // NOTE: theoretically, we should never hit this code path diff --git a/packages/astro/test/fixtures/i18n-server-island/astro.config.mjs b/packages/astro/test/fixtures/i18n-server-island/astro.config.mjs new file mode 100644 index 000000000000..cc445b3962c8 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-server-island/astro.config.mjs @@ -0,0 +1,17 @@ +import { defineConfig } from "astro/config"; + +export default defineConfig({ + i18n: { + defaultLocale: 'en', + locales: [ + 'en', 'pt', 'it', { + path: "spanish", + codes: ["es", "es-ar"] + } + ], + routing: { + prefixDefaultLocale: true + } + }, + base: "/new-site" +}) diff --git a/packages/astro/test/fixtures/i18n-server-island/package.json b/packages/astro/test/fixtures/i18n-server-island/package.json new file mode 100644 index 000000000000..e360d49d34c9 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-server-island/package.json @@ -0,0 +1,8 @@ +{ + "name": "@test/i18n-server-island", + "version": "0.0.0", + "private": true, + "dependencies": { + "astro": "workspace:*" + } +} diff --git a/packages/astro/test/fixtures/i18n-server-island/src/components/Island.astro b/packages/astro/test/fixtures/i18n-server-island/src/components/Island.astro new file mode 100644 index 000000000000..305caf85a9e1 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-server-island/src/components/Island.astro @@ -0,0 +1 @@ +

I am a server island

diff --git a/packages/astro/test/fixtures/i18n-server-island/src/pages/en/island.astro b/packages/astro/test/fixtures/i18n-server-island/src/pages/en/island.astro new file mode 100644 index 000000000000..b7bbf509bf7e --- /dev/null +++ b/packages/astro/test/fixtures/i18n-server-island/src/pages/en/island.astro @@ -0,0 +1,5 @@ +--- +import Island from "../../components/Island.astro" +--- + + diff --git a/packages/astro/test/fixtures/i18n-server-island/src/pages/index.astro b/packages/astro/test/fixtures/i18n-server-island/src/pages/index.astro new file mode 100644 index 000000000000..51507e040d25 --- /dev/null +++ b/packages/astro/test/fixtures/i18n-server-island/src/pages/index.astro @@ -0,0 +1,8 @@ + + + Astro + + +I am index + + diff --git a/packages/astro/test/i18n-routing.test.js b/packages/astro/test/i18n-routing.test.js index dc338c7db0b7..b5ecec60e1d7 100644 --- a/packages/astro/test/i18n-routing.test.js +++ b/packages/astro/test/i18n-routing.test.js @@ -2168,3 +2168,30 @@ describe('Fallback rewrite SSR', () => { assert.match(text, /Hello world/); }); }); + + +describe("i18n routing with server islands", () => { + /** @type {import('./test-utils').Fixture} */ + let fixture; + /** @type {import('./test-utils').DevServer} */ + let devServer; + + before(async () => { + fixture = await loadFixture({ + root: './fixtures/i18n-server-island/', + }); + devServer = await fixture.startDevServer(); + }); + + after(async () => { + await devServer.stop(); + }) + + it('should render the en locale with server island', async () => { + const res = await fixture.fetch('/en/island'); + const html = await res.text(); + const $ = cheerio.load(html); + const serverIslandScript = $('script[data-island-id]'); + assert.equal(serverIslandScript.length, 1, 'has the island script'); + }); +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 2b82d2cec166..92092a8994a4 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -3233,6 +3233,12 @@ importers: specifier: workspace:* version: link:../../.. + packages/astro/test/fixtures/i18n-server-island: + dependencies: + astro: + specifier: workspace:* + version: link:../../.. + packages/astro/test/fixtures/import-ts-with-js: dependencies: astro: From e21a6e81dd5512b0cac9e8e59d03fdd0cb20ef66 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Fri, 31 Jan 2025 16:15:04 +0000 Subject: [PATCH 2/2] remove unused import --- packages/astro/src/core/render-context.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/astro/src/core/render-context.ts b/packages/astro/src/core/render-context.ts index fb262ee657f4..70704d7b3d25 100644 --- a/packages/astro/src/core/render-context.ts +++ b/packages/astro/src/core/render-context.ts @@ -31,7 +31,6 @@ import { renderRedirect } from './redirects/render.js'; import { type Pipeline, Slots, getParams, getProps } from './render/index.js'; import { isRoute404or500, isRouteServerIsland } from './routing/match.js'; import { copyRequest, getOriginPathname, setOriginPathname } from './routing/rewrite.js'; -import { SERVER_ISLAND_COMPONENT } from './server-islands/endpoint.js'; import { AstroSession } from './session.js'; export const apiContextRoutesSymbol = Symbol.for('context.routes');