Skip to content

Commit

Permalink
feat(i18n): apply specific routing logic only to pages (#9091)
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico authored Nov 15, 2023
1 parent 60e8210 commit 536c6c9
Show file tree
Hide file tree
Showing 8 changed files with 115 additions and 6 deletions.
5 changes: 5 additions & 0 deletions .changeset/empty-turtles-wave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'astro': patch
---

The `routingStrategy` `prefix-always` should not force its logic to endpoints. This fixes some regression with `astro:assets` and `@astrojs/rss`.
3 changes: 2 additions & 1 deletion packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type {
SSRElement,
SSRManifest,
} from '../../@types/astro.js';
import { createI18nMiddleware } from '../../i18n/middleware.js';
import { createI18nMiddleware, i18nPipelineHook } from '../../i18n/middleware.js';
import type { SinglePageBuiltModule } from '../build/types.js';
import { getSetCookiesFromResponse } from '../cookies/index.js';
import { consoleLogDestination } from '../logger/console.js';
Expand Down Expand Up @@ -179,6 +179,7 @@ export class App {
} else {
this.#pipeline.setMiddlewareFunction(i18nMiddleware);
}
this.#pipeline.onBeforeRenderRoute(i18nPipelineHook);
} else {
if (mod.onRequest) {
this.#pipeline.setMiddlewareFunction(mod.onRequest as MiddlewareEndpointHandler);
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
removeLeadingForwardSlash,
removeTrailingForwardSlash,
} from '../../core/path.js';
import { createI18nMiddleware } from '../../i18n/middleware.js';
import { createI18nMiddleware, i18nPipelineHook } from '../../i18n/middleware.js';
import { runHookBuildGenerated } from '../../integrations/index.js';
import { getOutputDirectory, isServerLikeOutput } from '../../prerender/utils.js';
import { PAGE_SCRIPT_ID } from '../../vite-plugin-scripts/index.js';
Expand Down Expand Up @@ -289,6 +289,7 @@ async function generatePage(
} else {
pipeline.setMiddlewareFunction(i18nMiddleware);
}
pipeline.onBeforeRenderRoute(i18nPipelineHook);
} else if (onRequest) {
pipeline.setMiddlewareFunction(onRequest as MiddlewareEndpointHandler);
}
Expand Down
20 changes: 20 additions & 0 deletions packages/astro/src/core/pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@ type EndpointResultHandler = (
result: Response
) => Promise<Response> | Response;

type PipelineHooks = {
before: PipelineHookFunction[];
};

export type PipelineHookFunction = (ctx: RenderContext, mod: ComponentInstance | undefined) => void;

/**
* This is the basic class of a pipeline.
*
Expand All @@ -23,6 +29,9 @@ type EndpointResultHandler = (
export class Pipeline {
env: Environment;
#onRequest?: MiddlewareEndpointHandler;
#hooks: PipelineHooks = {
before: [],
};
/**
* The handler accepts the *original* `Request` and result returned by the endpoint.
* It must return a `Response`.
Expand Down Expand Up @@ -75,6 +84,9 @@ export class Pipeline {
renderContext: RenderContext,
componentInstance: ComponentInstance | undefined
): Promise<Response> {
for (const hook of this.#hooks.before) {
hook(renderContext, componentInstance);
}
const result = await this.#tryRenderRoute(
renderContext,
this.env,
Expand Down Expand Up @@ -158,4 +170,12 @@ export class Pipeline {
throw new Error(`Couldn't find route of type [${renderContext.route.type}]`);
}
}

/**
* Store a function that will be called before starting the rendering phase.
* @param fn
*/
onBeforeRenderRoute(fn: PipelineHookFunction) {
this.#hooks.before.push(fn);
}
}
27 changes: 24 additions & 3 deletions packages/astro/src/i18n/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import { appendForwardSlash, joinPaths } from '@astrojs/internal-helpers/path';
import type { MiddlewareEndpointHandler, SSRManifest } from '../@types/astro.js';
import type { MiddlewareEndpointHandler, RouteData, SSRManifest } from '../@types/astro.js';
import type { RouteInfo } from '../core/app/types.js';
import type { PipelineHookFunction } from '../core/pipeline.js';

const routeDataSymbol = Symbol.for('astro.routeData');

// Checks if the pathname doesn't have any locale, exception for the defaultLocale, which is ignored on purpose
function checkIsLocaleFree(pathname: string, locales: string[]): boolean {
Expand All @@ -26,9 +30,19 @@ export function createI18nMiddleware(
return await next();
}

const { locales, defaultLocale, fallback } = i18n;
const url = context.url;
const routeData = Reflect.get(context.request, routeDataSymbol);
if (routeData) {
// If the route we're processing is not a page, then we ignore it
if (
(routeData as RouteData).type !== 'page' &&
(routeData as RouteData).type !== 'fallback'
) {
return await next();
}
}

const url = context.url;
const { locales, defaultLocale, fallback } = i18n;
const response = await next();

if (response instanceof Response) {
Expand Down Expand Up @@ -83,3 +97,10 @@ export function createI18nMiddleware(
return response;
};
}

/**
* This pipeline hook attaches a `RouteData` object to the `Request`
*/
export const i18nPipelineHook: PipelineHookFunction = (ctx) => {
Reflect.set(ctx.request, routeDataSymbol, ctx.route);
};
3 changes: 2 additions & 1 deletion packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
import { createRequest } from '../core/request.js';
import { matchAllRoutes } from '../core/routing/index.js';
import { isPage, resolveIdToUrl } from '../core/util.js';
import { createI18nMiddleware } from '../i18n/middleware.js';
import { createI18nMiddleware, i18nPipelineHook } from '../i18n/middleware.js';
import { getSortedPreloadedMatches } from '../prerender/routing.js';
import { isServerLikeOutput } from '../prerender/utils.js';
import { PAGE_SCRIPT_ID } from '../vite-plugin-scripts/index.js';
Expand Down Expand Up @@ -289,6 +289,7 @@ export async function handleRoute({
} else {
pipeline.setMiddlewareFunction(i18Middleware);
}
pipeline.onBeforeRenderRoute(i18nPipelineHook);
} else if (onRequest) {
pipeline.setMiddlewareFunction(onRequest);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const GET = () => {
return new Response(JSON.stringify({ lorem: 'ipsum' }), {
headers: {
'content-type': 'application/json',
},
});
};
53 changes: 53 additions & 0 deletions packages/astro/test/i18-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -992,3 +992,56 @@ describe('[SSR] i18n routing', () => {
});
});
});

describe('i18n routing does not break assets and endpoints', () => {
describe('assets', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

before(async () => {
fixture = await loadFixture({
root: './fixtures/core-image-base/',
experimental: {
i18n: {
defaultLocale: 'en',
locales: ['en', 'es'],
},
},
base: '/blog',
});
await fixture.build();
});

it('should render the image', async () => {
const html = await fixture.readFile('/index.html');
const $ = cheerio.load(html);
const src = $('#local img').attr('src');
expect(src.length).to.be.greaterThan(0);
expect(src.startsWith('/blog')).to.be.true;
});
});

describe('endpoint', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

let app;

before(async () => {
fixture = await loadFixture({
root: './fixtures/i18n-routing-prefix-always/',
output: 'server',
adapter: testAdapter(),
});
await fixture.build();
app = await fixture.loadTestAdapterApp();
});

it('should return the expected data', async () => {
let request = new Request('http://example.com/new-site/test.json');
let response = await app.render(request);
expect(response.status).to.equal(200);
expect(await response.text()).includes('lorem');
});
});
});

0 comments on commit 536c6c9

Please sign in to comment.