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

refactor: unsplit astro:i18n module #9790

Merged
merged 4 commits into from
Jan 24, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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/tame-crabs-reply.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"astro": patch
---

Refactors internals of the `astro:i18n` module to be more maintainable.
142 changes: 1 addition & 141 deletions packages/astro/client.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,147 +149,7 @@ declare module 'astro:prefetch' {
}

declare module 'astro:i18n' {
export type GetLocaleOptions = import('./dist/virtual-modules/i18n.js').GetLocaleOptions;

/**
* @param {string} locale A locale
* @param {string} [path=""] An optional path to add after the `locale`.
* @param {import('./dist/virtual-modules/i18n.js').GetLocaleOptions} options Customise the generated path
* @return {string}
*
* Returns a _relative_ path with passed locale.
*
* ## Errors
*
* Throws an error if the locale doesn't exist in the list of locales defined in the configuration.
*
* ## Examples
*
* ```js
* import { getRelativeLocaleUrl } from "astro:i18n";
* getRelativeLocaleUrl("es"); // /es
* getRelativeLocaleUrl("es", "getting-started"); // /es/getting-started
* getRelativeLocaleUrl("es_US", "getting-started", { prependWith: "blog" }); // /blog/es-us/getting-started
* getRelativeLocaleUrl("es_US", "getting-started", { prependWith: "blog", normalizeLocale: false }); // /blog/es_US/getting-started
* ```
*/
export const getRelativeLocaleUrl: (
locale: string,
path?: string,
options?: GetLocaleOptions
) => string;

/**
*
* @param {string} locale A locale
* @param {string} [path=""] An optional path to add after the `locale`.
* @param {import('./dist/virtual-modules/i18n.js').GetLocaleOptions} options Customise the generated path
* @return {string}
*
* Returns an absolute path with the passed locale. The behaviour is subject to change based on `site` configuration.
* If _not_ provided, the function will return a _relative_ URL.
*
* ## Errors
*
* Throws an error if the locale doesn't exist in the list of locales defined in the configuration.
*
* ## Examples
*
* If `site` is `https://example.com`:
*
* ```js
* import { getAbsoluteLocaleUrl } from "astro:i18n";
* getAbsoluteLocaleUrl("es"); // https://example.com/es
* getAbsoluteLocaleUrl("es", "getting-started"); // https://example.com/es/getting-started
* getAbsoluteLocaleUrl("es_US", "getting-started", { prependWith: "blog" }); // https://example.com/blog/es-us/getting-started
* getAbsoluteLocaleUrl("es_US", "getting-started", { prependWith: "blog", normalizeLocale: false }); // https://example.com/blog/es_US/getting-started
* ```
*/
export const getAbsoluteLocaleUrl: (
locale: string,
path?: string,
options?: GetLocaleOptions
) => string;

/**
* @param {string} [path=""] An optional path to add after the `locale`.
* @param {import('./dist/virtual-modules/i18n.js').GetLocaleOptions} options Customise the generated path
* @return {string[]}
*
* Works like `getRelativeLocaleUrl` but it emits the relative URLs for ALL locales:
*/
export const getRelativeLocaleUrlList: (path?: string, options?: GetLocaleOptions) => string[];
/**
* @param {string} [path=""] An optional path to add after the `locale`.
* @param {import('./dist/virtual-modules/i18n.js').GetLocaleOptions} options Customise the generated path
* @return {string[]}
*
* Works like `getAbsoluteLocaleUrl` but it emits the absolute URLs for ALL locales:
*/
export const getAbsoluteLocaleUrlList: (path?: string, options?: GetLocaleOptions) => string[];

/**
* A function that return the `path` associated to a locale (defined as code). It's particularly useful in case you decide
* to use locales that are broken down in paths and codes.
*
* @param {string} code The code of the locale
* @returns {string} The path associated to the locale
*
* ## Example
*
* ```js
* // astro.config.mjs
*
* export default defineConfig({
* i18n: {
* locales: [
* { codes: ["it", "it-VT"], path: "italiano" },
* "es"
* ]
* }
* })
* ```
*
* ```js
* import { getPathByLocale } from "astro:i18n";
* getPathByLocale("it"); // returns "italiano"
* getPathByLocale("it-VT"); // returns "italiano"
* getPathByLocale("es"); // returns "es"
* ```
*/
export const getPathByLocale: (code: string) => string;

/**
* A function that returns the preferred locale given a certain path. This is particularly useful if you configure a locale using
* `path` and `codes`. When you define multiple `code`, this function will return the first code of the array.
*
* Astro will treat the first code as the one that the user prefers.
*
* @param {string} path The path that maps to a locale
* @returns {string} The path associated to the locale
*
* ## Example
*
* ```js
* // astro.config.mjs
*
* export default defineConfig({
* i18n: {
* locales: [
* { codes: ["it-VT", "it"], path: "italiano" },
* "es"
* ]
* }
* })
* ```
*
* ```js
* import { getLocaleByPath } from "astro:i18n";
* getLocaleByPath("italiano"); // returns "it-VT" because that's the first code configured
* getLocaleByPath("es"); // returns "es"
* ```
*/
export const getLocaleByPath: (path: string) => string;
export * from 'astro/virtual-modules/i18n.js';
}

declare module 'astro:middleware' {
Expand Down
2 changes: 1 addition & 1 deletion packages/astro/src/core/create-vite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ export async function createVite(
astroTransitions({ settings }),
astroDevToolbar({ settings, logger }),
vitePluginFileURL({}),
!!settings.config.i18n && astroInternationalization({ settings }),
astroInternationalization({ settings }),
],
publicDir: fileURLToPath(settings.config.publicDir),
root: fileURLToPath(settings.config.root),
Expand Down
29 changes: 23 additions & 6 deletions packages/astro/src/i18n/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export function getLocaleAbsoluteUrl({ site, ...rest }: GetLocaleAbsoluteUrl) {
}
}

type GetLocalesBaseUrl = GetLocaleOptions & {
interface GetLocalesRelativeUrlList extends GetLocaleOptions {
base: string;
locales: Locales;
trailingSlash: AstroConfig['trailingSlash'];
Expand All @@ -103,7 +103,7 @@ export function getLocaleRelativeUrlList({
normalizeLocale = false,
routing = 'pathname-prefix-other-locales',
defaultLocale,
}: GetLocalesBaseUrl) {
}: GetLocalesRelativeUrlList) {
const locales = toPaths(_locales);
return locales.map((locale) => {
const pathsToJoin = [base, prependWith];
Expand All @@ -123,7 +123,11 @@ export function getLocaleRelativeUrlList({
});
}

export function getLocaleAbsoluteUrlList({ site, ...rest }: GetLocaleAbsoluteUrl) {
interface GetLocalesAbsoluteUrlList extends GetLocalesRelativeUrlList {
site?: string
}

export function getLocaleAbsoluteUrlList({ site, ...rest }: GetLocalesAbsoluteUrlList) {
const locales = getLocaleRelativeUrlList(rest);
return locales.map((locale) => {
if (site) {
Expand All @@ -139,7 +143,7 @@ export function getLocaleAbsoluteUrlList({ site, ...rest }: GetLocaleAbsoluteUrl
* @param locale
* @param locales
*/
export function getPathByLocale(locale: string, locales: Locales) {
export function getPathByLocale(locale: string, locales: Locales): string {
for (const loopLocale of locales) {
if (typeof loopLocale === 'string') {
if (loopLocale === locale) {
Expand All @@ -153,6 +157,7 @@ export function getPathByLocale(locale: string, locales: Locales) {
}
}
}
throw new Unreachable();
}

/**
Expand All @@ -161,19 +166,20 @@ export function getPathByLocale(locale: string, locales: Locales) {
* @param path
* @param locales
*/
export function getLocaleByPath(path: string, locales: Locales): string | undefined {
export function getLocaleByPath(path: string, locales: Locales): string {
for (const locale of locales) {
if (typeof locale !== 'string') {
if (locale.path === path) {
// the first code is the one that user usually wants
const code = locale.codes.at(0);
if (code === undefined) throw new Unreachable();
return code;
}
} else if (locale === path) {
return locale;
}
}
return undefined;
throw new Unreachable();
}

/**
Expand Down Expand Up @@ -235,3 +241,14 @@ function peekCodePathToUse(locales: Locales, locale: string): undefined | string

return undefined;
}

class Unreachable extends Error {
constructor() {
super(
"Astro encountered an unexpected line of code.\n" +
"In most cases, this is not your fault, but a bug in astro code.\n" +
"If there isn't one already, please create an issue.\n" +
"https://astro.build/issues"
);
}
}
24 changes: 8 additions & 16 deletions packages/astro/src/i18n/middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,27 +29,19 @@ export function createI18nMiddleware(
base: SSRManifest['base'],
trailingSlash: SSRManifest['trailingSlash'],
buildFormat: SSRManifest['buildFormat']
): MiddlewareHandler | undefined {
if (!i18n) {
return undefined;
}
): MiddlewareHandler {
if (!i18n) return (_, next) => next();

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

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, routing } = i18n;
const response = await next();
Expand Down
60 changes: 13 additions & 47 deletions packages/astro/src/i18n/vite-plugin-i18n.ts
Original file line number Diff line number Diff line change
@@ -1,69 +1,35 @@
import type * as vite from 'vite';
import type { AstroSettings } from '../@types/astro.js';
import type { AstroConfig, AstroSettings } from '../@types/astro.js';

const virtualModuleId = 'astro:i18n';
const resolvedVirtualModuleId = '\0' + virtualModuleId;
const configId = 'astro-internal:i18n-config';
const resolvedConfigId = `\0${configId}`;

type AstroInternationalization = {
settings: AstroSettings;
};

export interface I18nInternalConfig extends Pick<AstroConfig, 'base' | 'site' | 'trailingSlash'>, NonNullable<AstroConfig['i18n']>, Pick<AstroConfig["build"], "format"> {}

export default function astroInternationalization({
settings,
}: AstroInternationalization): vite.Plugin {
const { base, build: { format }, i18n, site, trailingSlash } = settings.config;
return {
name: 'astro:i18n',
enforce: 'pre',
async resolveId(id) {
if (id === virtualModuleId) {
return resolvedVirtualModuleId;
if (i18n === undefined) throw new Error("The `astro:i18n` module can not be used without enabling i18n in your Astro config.");
return this.resolve("astro/virtual-modules/i18n.js");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error here is new. Previously, it would throw with a generic "cant resolve" message.

Copy link
Member

Choose a reason for hiding this comment

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

Should we throw an AstroError instead? Or use this.error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if one of them is better. I can say this will properly display the message in the browser error overlay.

Copy link
Member

@Princesseuh Princesseuh Jan 23, 2024

Choose a reason for hiding this comment

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

I think this should be an AstroError, yes! I didn't fully understand the this.error thing though

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be an AstroError, yes! I didn't fully understand the this.error thing though

In a rollup plugin you can use this.error to throw an error without breaking too much stuff :D

Copy link
Contributor Author

@lilnasy lilnasy Jan 23, 2024

Choose a reason for hiding this comment

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

this.error()

image

throw new Error()

image

throw new AstroError()

image

Copy link
Member

Choose a reason for hiding this comment

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

I'd go with a AstroError so that we can have docs and stuff on it then!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image

}
if (id === configId) return resolvedConfigId;
},
load(id) {
if (id === resolvedVirtualModuleId) {
return `
import {
getLocaleRelativeUrl as _getLocaleRelativeUrl,
getLocaleRelativeUrlList as _getLocaleRelativeUrlList,
getLocaleAbsoluteUrl as _getLocaleAbsoluteUrl,
getLocaleAbsoluteUrlList as _getLocaleAbsoluteUrlList,
getPathByLocale as _getPathByLocale,
getLocaleByPath as _getLocaleByPath,
} from "astro/virtual-modules/i18n.js";

const base = ${JSON.stringify(settings.config.base)};
const trailingSlash = ${JSON.stringify(settings.config.trailingSlash)};
const format = ${JSON.stringify(settings.config.build.format)};
const site = ${JSON.stringify(settings.config.site)};
const i18n = ${JSON.stringify(settings.config.i18n)};

export const getRelativeLocaleUrl = (locale, path = "", opts) => _getLocaleRelativeUrl({
locale,
path,
base,
trailingSlash,
format,
...i18n,
...opts
});
export const getAbsoluteLocaleUrl = (locale, path = "", opts) => _getLocaleAbsoluteUrl({
locale,
path,
base,
trailingSlash,
format,
site,
...i18n,
...opts
});

export const getRelativeLocaleUrlList = (path = "", opts) => _getLocaleRelativeUrlList({
base, path, trailingSlash, format, ...i18n, ...opts });
export const getAbsoluteLocaleUrlList = (path = "", opts) => _getLocaleAbsoluteUrlList({ base, path, trailingSlash, format, site, ...i18n, ...opts });

export const getPathByLocale = (locale) => _getPathByLocale(locale, i18n.locales);
export const getLocaleByPath = (path) => _getLocaleByPath(path, i18n.locales);
`;
if (id === resolvedConfigId) {
const { defaultLocale, locales, routing, fallback } = i18n!;
const config: I18nInternalConfig = { base, format, site, trailingSlash, defaultLocale, locales, routing, fallback };
return `export default ${JSON.stringify(config)};`;
Comment on lines +32 to +34
Copy link
Member

Choose a reason for hiding this comment

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

Definitely more maintainable!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep, and this will be the key to removing i18n from the manifest

}
},
};
Expand Down
Loading