Skip to content

Commit

Permalink
feat(i18n): apply specific routing logic only to pages
Browse files Browse the repository at this point in the history
  • Loading branch information
ematipico committed Nov 13, 2023
1 parent 22e7c2f commit c79944e
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 21 deletions.
7 changes: 3 additions & 4 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,9 @@ export class App {
let i18nMiddleware = createI18nMiddleware(
this.#manifest.i18n,
this.#manifest.base,
this.#manifest.routes,
this.#manifest.trailingSlash

);
this.#manifest.trailingSlash,
this.#manifest.routes.map((route) => route.routeData)
);
if (i18nMiddleware) {
if (mod.onRequest) {
this.#pipeline.setMiddlewareFunction(
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ async function generatePage(
pipeline.getManifest().i18n,
pipeline.getManifest().base,
pipeline.getManifest().trailingSlash,
pipeline.getManifest().routes
pipeline.getManifest().routes.map((route) => route.routeData)
);
if (config.experimental.i18n && i18nMiddleware) {
if (onRequest) {
Expand Down
42 changes: 30 additions & 12 deletions packages/astro/src/i18n/middleware.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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';

// Checks if the pathname doesn't have any locale, exception for the defaultLocale, which is ignored on purpose
Expand All @@ -17,15 +17,15 @@ export function createI18nMiddleware(
i18n: SSRManifest['i18n'],
base: SSRManifest['base'],
trailingSlash: SSRManifest['trailingSlash'],

routes: SSRManifest['routes']
routes: RouteData[]
): MiddlewareEndpointHandler | undefined {
if (!i18n) {
return undefined;
}

const pageRoutes = routes.filter((route) => {
return route.routeData.type === 'page';
// we get all the routes that aren't pages
const nonPagesRoutes = routes.filter((route) => {
return route.type !== 'page';
});

return async (context, next) => {
Expand All @@ -34,8 +34,12 @@ export function createI18nMiddleware(
}

const url = context.url;
if (!isPathnameARoutePage(pageRoutes, url.pathname)) {
return await next();
// We get the pathname
// Internally, Astro removes the `base` from the manifest data of the routes.
// We have to make sure that we remove it from the pathname of the request
let astroPathname = url.pathname;
if (astroPathname.startsWith(base) && base !== '/') {
astroPathname = astroPathname.slice(base.length);
}

const { locales, defaultLocale, fallback } = i18n;
Expand All @@ -54,6 +58,11 @@ export function createI18nMiddleware(
headers: response.headers,
});
} else if (i18n.routingStrategy === 'prefix-always') {
// We want to do this check only here, because `prefix-other-locales` assumes that non localized folder are valid
if (shouldSkipRoute(astroPathname, nonPagesRoutes, locales)) {
return await next();
}

if (url.pathname === base + '/' || url.pathname === base) {
if (trailingSlash === 'always') {
return context.redirect(`${appendForwardSlash(joinPaths(base, i18n.defaultLocale))}`);
Expand Down Expand Up @@ -96,12 +105,21 @@ export function createI18nMiddleware(
}

/**
* Check whether the pathname of the current request belongs to a route of type page.
* @param pageRoutes
* @param pathname
* Checks whether a route should be skipped from the middleware logic. A route should be not be skipped when:
* - it's the home
* - it contains any locale
* - the pathname belongs to a route that is not a page
*/
function isPathnameARoutePage(pageRoutes: RouteInfo[], pathname: string) {
function shouldSkipRoute(pathname: string, pageRoutes: RouteData[], locales: string[]) {
if (!pathname.length || pathname === '/') {
return false;
}

if (locales.some((locale) => pathname.includes(`/${locale}`))) {
return false;
}

return pageRoutes.some((route) => {
return route.routeData.route === pathname;
return !route.pattern.test(pathname);
});
}
5 changes: 2 additions & 3 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,9 +281,8 @@ export async function handleRoute({
config.experimental.i18n,
config.base,
config.trailingSlash,
manifest.routes

);
manifestData.routes
);

if (i18Middleware) {
if (onRequest) {
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/test/i18-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1038,7 +1038,7 @@ describe('i18n routing does not break assets and endpoints', () => {
});

it('should return the expected data', async () => {
let request = new Request('http://example.com/new-site/api/test');
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');
Expand Down

0 comments on commit c79944e

Please sign in to comment.