Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(i18n): server island request #13112

Merged
merged 2 commits into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/silent-worms-whisper.md
Original file line number Diff line number Diff line change
@@ -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`
5 changes: 2 additions & 3 deletions packages/astro/src/core/render-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@ 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';

export const apiContextRoutesSymbol = Symbol.for('context.routes');
Expand Down Expand Up @@ -596,7 +595,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)) {
Expand Down
39 changes: 39 additions & 0 deletions packages/astro/src/core/routing/match.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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);
}
1 change: 1 addition & 0 deletions packages/astro/src/core/server-islands/endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SSRManifest, 'base' | 'trailingSlash'>;

Expand Down
7 changes: 0 additions & 7 deletions packages/astro/src/i18n/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { REROUTE_DIRECTIVE_HEADER } from '../core/constants.js';
import { MissingLocale, i18nNoLocaleFoundInPath } from '../core/errors/errors-data.js';
import { AstroError } from '../core/errors/index.js';
import { isRoute404, isRoute500 } from '../core/routing/match.js';

Check warning on line 7 in packages/astro/src/i18n/index.ts

View workflow job for this annotation

GitHub Actions / Lint

lint/correctness/noUnusedImports

This import is unused.

Check warning on line 7 in packages/astro/src/i18n/index.ts

View workflow job for this annotation

GitHub Actions / Lint

lint/correctness/noUnusedImports

This import is unused.
import type { AstroConfig, Locales, ValidRedirectStatus } from '../types/public/config.js';
import type { APIContext } from '../types/public/context.js';
import { createI18nMiddleware } from './middleware.js';
Expand All @@ -16,13 +16,6 @@
};
}

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('/');
Expand Down
8 changes: 7 additions & 1 deletion packages/astro/src/i18n/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'],
Expand Down Expand Up @@ -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
Expand Down
17 changes: 17 additions & 0 deletions packages/astro/test/fixtures/i18n-server-island/astro.config.mjs
Original file line number Diff line number Diff line change
@@ -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"
})
8 changes: 8 additions & 0 deletions packages/astro/test/fixtures/i18n-server-island/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/i18n-server-island",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p>I am a server island</p>
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
import Island from "../../components/Island.astro"
---

<Island server:defer />
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
I am index
</body>
</html>
27 changes: 27 additions & 0 deletions packages/astro/test/i18n-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
})
6 changes: 6 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.