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

feat(i18n): domain with lookup table #9112

Merged
merged 7 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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
10 changes: 7 additions & 3 deletions packages/astro/src/@types/astro.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1520,7 +1520,7 @@ export interface AstroUserConfig {
* @docs
* @kind h4
* @name experimental.i18n.routingStrategy
* @type {'prefix-always' | 'prefix-other-locales'}
* @type {'prefix-always' | 'prefix-other-locales' | 'domain'}
* @default 'prefix-other-locales'
* @version 3.5.0
* @description
Expand All @@ -1533,9 +1533,12 @@ export interface AstroUserConfig {
* - `prefix-always`: All URLs will display a language prefix.
* URLs will be of the form `example.com/[locale]/content/` for every route, including the default language.
* Localized folders are used for every language, including the default.
* - `domain`: SSR only, it enables support for different domains. When a locale is mapped to domain, all the URLs won't have the language prefix.
* You map `fr` to `fr.example.com`, if you want a to have a blog page to look like `fr.example.com/blog` instead of `example.com/fr/blog`.
* The localised folders be must in the `src/pages/` folder.
Comment on lines +1536 to +1538
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have some questions about how this works to be sure I understand:

  1. if you select routingStrategy: "domain", what happens to the URLs for your defaultLocale? Does it depend on whether you're using a /[defaultLocale]/ folder structure? Does it make any assumptions by default?
  2. Is it correct to assume that for non-default locales, which will all be written in /locale/ folders, that the URLs will be prefixed if you don't specifically add them to domains? So, it's possible for a site to have a mixture of e.g. example.com/fr/ and es.example.com?
  3. is domains ignored if routingStrategy is set to one of the two other options? Could it cause problems to have domains configured if you do NOT have domain set as your routing strategy?
  4. Does the order of domains and routingStrategy matter in the i18n configuration? Could someone run into trouble if these are not in the proper order?
  5. Looking at the example below, routingStrategy is outside of the object containing the other config options. In the docs, we show it as inside this object. Can you confirm which should be correct?

Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yeah, there are assumptions. If you want to map a locale to a domain, the locale folder must be at src/pages/[locale]. The same goes for the defaultLocale. If the default locale isn't mapped to a domain, there aren't any assumptions. Although we are still evaluating the behaviour of defaultLocale, so it's possible we will change it soon.
  2. Yes, that is correct.
  3. No, that will be an error. For example, if you have i18n.domains and routingStategy: "prefix-always", Astro will throw a validation nerror.
  4. No, the order doesn't matter.
  5. Mmmmh, routingStategy is inside the i18n, same goes for domains. Are you referring to something else?

*
*/
routingStrategy?: 'prefix-always' | 'prefix-other-locales';
routingStrategy?: 'prefix-always' | 'prefix-other-locales' | 'domain';

/**
* @docs
Expand All @@ -1556,7 +1559,8 @@ export interface AstroUserConfig {
* locales: ["en", "fr", "pt-br", "es"],
* domains: {
* fr: "https://fr.example.com",
* }
* },
* routingStrategy: "domain"
* }
* }
* })
Expand Down
30 changes: 28 additions & 2 deletions packages/astro/src/core/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ import { consoleLogDestination } from '../logger/console.js';
import { AstroIntegrationLogger, Logger } from '../logger/core.js';
import { sequence } from '../middleware/index.js';
import {
appendForwardSlash,
collapseDuplicateSlashes,
joinPaths,
prependForwardSlash,
removeTrailingForwardSlash,
} from '../path.js';
Expand All @@ -28,6 +30,7 @@ import {
import { matchRoute } from '../routing/match.js';
import { EndpointNotFoundError, SSRRoutePipeline } from './ssrPipeline.js';
import type { RouteInfo } from './types.js';
import { shouldAppendForwardSlash } from '../build/util.js';
export { deserializeManifest } from './common.js';

const clientLocalsSymbol = Symbol.for('astro.locals');
Expand Down Expand Up @@ -131,8 +134,31 @@ export class App {
const url = new URL(request.url);
// ignore requests matching public assets
if (this.#manifest.assets.has(url.pathname)) return undefined;
const pathname = prependForwardSlash(this.removeBase(url.pathname));
const routeData = matchRoute(pathname, this.#manifestData);
let pathname;
if (this.#manifest.i18n && this.#manifest.i18n.routingStrategy === 'domain') {
ematipico marked this conversation as resolved.
Show resolved Hide resolved
const host = request.headers.get('X-Forwarded-Host');
ematipico marked this conversation as resolved.
Show resolved Hide resolved
if (host) {
try {
const locale = this.#manifest.i18n.domainLookupTable[host];
ematipico marked this conversation as resolved.
Show resolved Hide resolved
if (locale) {
pathname = prependForwardSlash(joinPaths(locale, this.removeBase(url.pathname)));
if (url.pathname.endsWith('/')) {
pathname = appendForwardSlash(pathname);
}
}
} catch (e) {
// waiting to decide what to do here
// eslint-disable-next-line no-console
console.error(e);
// TODO: What kind of error should we try? This happens if we have an invalid value inside the X-Forwarded-Host header
}
}
}
if (!pathname) {
pathname = prependForwardSlash(this.removeBase(url.pathname));
}
let routeData = matchRoute(pathname, this.#manifestData);

// missing routes fall-through, prerendered are handled by static layer
if (!routeData || routeData.prerender) return undefined;
return routeData;
Expand Down
3 changes: 2 additions & 1 deletion packages/astro/src/core/app/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,10 @@ export type SSRManifest = {

export type SSRManifestI18n = {
fallback?: Record<string, string>;
routingStrategy?: 'prefix-always' | 'prefix-other-locales';
routingStrategy?: 'prefix-always' | 'prefix-other-locales' | 'domain';
locales: string[];
defaultLocale: string;
domainLookupTable: Record<string, string>;
};

export type SerializedSSRManifest = Omit<
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/core/build/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,7 @@ export function createBuildManifest(
routingStrategy: settings.config.experimental.i18n.routingStrategy,
defaultLocale: settings.config.experimental.i18n.defaultLocale,
locales: settings.config.experimental.i18n.locales,
domainLookupTable: {},
};
}
return {
Expand Down
16 changes: 15 additions & 1 deletion packages/astro/src/core/build/plugins/plugin-manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,15 @@ import type {
SerializedRouteInfo,
SerializedSSRManifest,
} from '../../app/types.js';
import { joinPaths, prependForwardSlash } from '../../path.js';
import { appendForwardSlash, joinPaths, prependForwardSlash } from '../../path.js';
import { serializeRouteData } from '../../routing/index.js';
import { addRollupInput } from '../add-rollup-input.js';
import { getOutFile, getOutFolder } from '../common.js';
import { cssOrder, mergeInlineCss, type BuildInternals } from '../internal.js';
import type { AstroBuildPlugin } from '../plugin.js';
import type { StaticBuildOptions } from '../types.js';
import { shouldAppendForwardSlash } from '../util.js';
import { normalizeTheLocale } from '../../../i18n/index.js';

const manifestReplace = '@@ASTRO_MANIFEST_REPLACE@@';
const replaceExp = new RegExp(`['"](${manifestReplace})['"]`, 'g');
Expand Down Expand Up @@ -161,6 +163,7 @@ function buildManifest(
const { settings } = opts;

const routes: SerializedRouteInfo[] = [];
const domainLookupTable: Record<string, string> = {};
const entryModules = Object.fromEntries(internals.entrySpecifierToBundleMap.entries());
if (settings.scripts.some((script) => script.stage === 'page')) {
staticFiles.push(entryModules[PAGE_SCRIPT_ID]);
Expand Down Expand Up @@ -236,6 +239,16 @@ function buildManifest(
});
}

/**
* logic meant for i18n domain support, where we fill the lookup table
*/
const i18n = settings.config.experimental.i18n;
if (i18n && i18n.domains && i18n.routingStrategy === 'domain') {
for (const [locale, domainValue] of Object.entries(i18n.domains)) {
domainLookupTable[domainValue] = normalizeTheLocale(locale);
}
}

// HACK! Patch this special one.
if (!(BEFORE_HYDRATION_SCRIPT_ID in entryModules)) {
// Set this to an empty string so that the runtime knows not to try and load this.
Expand All @@ -248,6 +261,7 @@ function buildManifest(
routingStrategy: settings.config.experimental.i18n.routingStrategy,
locales: settings.config.experimental.i18n.locales,
defaultLocale: settings.config.experimental.i18n.defaultLocale,
domainLookupTable,
};
}

Expand Down
15 changes: 12 additions & 3 deletions packages/astro/src/core/config/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -356,16 +356,15 @@ export const AstroConfigSchema = z.object({
)
)
.optional(),
// TODO: properly add default when the feature goes of experimental
routingStrategy: z
.enum(['prefix-always', 'prefix-other-locales'])
.enum(['prefix-always', 'prefix-other-locales', 'domain'])
.optional()
.default('prefix-other-locales'),
})
.optional()
.superRefine((i18n, ctx) => {
if (i18n) {
const { defaultLocale, locales, fallback, domains } = i18n;
const { defaultLocale, locales, fallback, domains, routingStrategy } = i18n;
if (!locales.includes(defaultLocale)) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
Expand Down Expand Up @@ -397,6 +396,16 @@ export const AstroConfigSchema = z.object({
}
}
if (domains) {
const entries = Object.entries(domains);
if (entries.length > 0) {
if (routingStrategy !== 'domain') {
ctx.addIssue({
code: z.ZodIssueCode.custom,
message: `When specifying some domains, the property \`i18n.routingStrategy\` must be set to \`"domain"\`.`,
});
}
}

for (const [domainKey, domainValue] of Object.entries(domains)) {
if (!locales.includes(domainKey)) {
ctx.addIssue({
Expand Down
1 change: 1 addition & 0 deletions packages/astro/src/vite-plugin-astro-server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
routingStrategy: settings.config.experimental.i18n.routingStrategy,
defaultLocale: settings.config.experimental.i18n.defaultLocale,
locales: settings.config.experimental.i18n.locales,
domainLookupTable: {},
};
}
return {
Expand Down
6 changes: 1 addition & 5 deletions packages/astro/src/vite-plugin-astro-server/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -277,11 +277,7 @@ export async function handleRoute({

const onRequest = middleware?.onRequest as MiddlewareEndpointHandler | undefined;
if (config.experimental.i18n) {
const i18Middleware = createI18nMiddleware(
config.experimental.i18n,
config.base,
config.trailingSlash
);
const i18Middleware = createI18nMiddleware(manifest.i18n, config.base, config.trailingSlash);

if (i18Middleware) {
if (onRequest) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import { defineConfig} from "astro/config";

export default defineConfig({
trailingSlash: "never",
experimental: {
i18n: {
defaultLocale: 'en',
locales: [
'en', 'pt', 'it'
],
domains: {
pt: "https://example.pt"
},
routingStrategy: "domain"
},

},
base: "/new-site"
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"name": "@test/i18n-routing-subdomain",
"version": "0.0.0",
"private": true,
"dependencies": {
"astro": "workspace:*"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
export function getStaticPaths() {
return [
{params: {id: '1'}, props: { content: "Hello world" }},
{params: {id: '2'}, props: { content: "Eat Something" }},
{params: {id: '3'}, props: { content: "How are you?" }},
];
}
const { content } = Astro.props;
---
<html>
<head>
<title>Astro</title>
</head>
<body>
{content}
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Hello
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Start
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
---
export function getStaticPaths() {
return [
{params: {id: '1'}, props: { content: "Hola mundo" }},
{params: {id: '2'}, props: { content: "Eat Something" }},
{params: {id: '3'}, props: { content: "How are you?" }},
];
}
const { content } = Astro.props;
---
<html>
<head>
<title>Astro</title>
</head>
<body>
{content}
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<html>
<head>
<title>Astro</title>
</head>
<body>
Oi essa e start
</body>
</html>
3 changes: 2 additions & 1 deletion packages/astro/test/fixtures/i18n-routing/astro.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ export default defineConfig({
],
domains: {
it: "https://it.example.com"
}
},
routingStrategy: "domain"
}
},
})
26 changes: 26 additions & 0 deletions packages/astro/test/i18-routing.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1009,4 +1009,30 @@ describe('[SSR] i18n routing', () => {
});
});
});

describe('i18n routing with routing strategy [subdomain]', () => {
/** @type {import('./test-utils').Fixture} */
let fixture;

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

it('should render the en locale', async () => {
let request = new Request('http://example.pt/new-site/start', {
headers: {
'X-Forwarded-Host': 'https://example.pt',
},
});
let response = await app.render(request);
expect(response.status).to.equal(200);
expect(await response.text()).includes('Oi essa e start\n');
});
});
});
24 changes: 24 additions & 0 deletions packages/astro/test/units/config/config-validate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,7 @@ describe('Config Validation', () => {
domains: {
lorem: 'https://example.com',
},
routingStrategy: 'domain',
},
},
},
Expand All @@ -191,6 +192,7 @@ describe('Config Validation', () => {
domains: {
en: 'www.example.com',
},
routingStrategy: 'domain',
},
},
},
Expand All @@ -212,6 +214,7 @@ describe('Config Validation', () => {
domains: {
en: 'https://www.example.com/blog/page/',
},
routingStrategy: 'domain',
},
},
},
Expand All @@ -222,5 +225,26 @@ describe('Config Validation', () => {
"The URL `https://www.example.com/blog/page/` must contain only the origin. A subsequent pathname isn't allowed here. Remove `/blog/page/`."
);
});

it('errors if there are domains, and the routing strategy is not correct', async () => {
const configError = await validateConfig(
{
experimental: {
i18n: {
defaultLocale: 'en',
locales: ['es', 'en'],
domains: {
en: 'https://www.example.com/',
},
},
},
},
process.cwd()
).catch((err) => err);
expect(configError instanceof z.ZodError).to.equal(true);
expect(configError.errors[0].message).to.equal(
'When specifying some domains, the property `i18n.routingStrategy` must be set to `"domain"`.'
);
});
});
});
Loading
Loading