chore: switch formatter to format-message (#4088)

Co-authored-by: Robert Korulczyk <[email protected]>

framework/core/js/package.json

@@ -5,14 +5,13 @@
     "type": "module",
     "prettier": "@flarum/prettier-config",
     "dependencies": {
-        "@askvortsov/rich-icu-message-formatter": "^0.2.4",
-        "@ultraq/icu-message-formatter": "^0.12.0",
         "body-scroll-lock": "^4.0.0-beta.0",
         "bootstrap": "^3.4.1",
         "clsx": "^1.1.1",
         "color-thief-browser": "^2.0.2",
         "dayjs": "^1.10.7",
         "focus-trap": "^6.7.1",
+        "format-message": "^6.2.4",
         "jquery": "^3.6.0",
         "jquery.hotkeys": "^0.1.0",
         "mithril": "^2.2",

framework/core/js/src/common/Translator.tsx

@@ -1,21 +1,23 @@
-import type { Dayjs } from 'dayjs';
-import { RichMessageFormatter, mithrilRichHandler, NestedStringArray } from '@askvortsov/rich-icu-message-formatter';
-import { pluralTypeHandler, selectTypeHandler } from '@ultraq/icu-message-formatter';
 import username from './helpers/username';
+import type { Dayjs } from 'dayjs';
 import User from './models/User';
 import extract from './utils/extract';
+import formatMessage, { Translation } from 'format-message';
+import fireDebugWarning from './helpers/fireDebugWarning';
 import extractText from './utils/extractText';
 import ItemList from './utils/ItemList';
 
-type Translations = Record<string, string>;
+type Translations = { [key: string]: string | Translation };
 type TranslatorParameters = Record<string, unknown>;
 type DateTimeFormatCallback = (id?: string) => string | void;
 
 export default class Translator {
   /**
    * A map of translation keys to their translated values.
    */
-  translations: Translations = {};
+  get translations(): Translations {
+    return this.formatter.setup().translations[this.getLocale()] ?? {};
+  }
 
   /**
    * A item list of date time format callbacks.
@@ -25,44 +27,44 @@ export default class Translator {
   /**
    * The underlying ICU MessageFormatter util.
    */
-  protected formatter = new RichMessageFormatter(null, this.formatterTypeHandlers(), mithrilRichHandler);
+  protected formatter = formatMessage;
 
   /**
    * Sets the formatter's locale to the provided value.
    */
   setLocale(locale: string) {
-    this.formatter.locale = locale;
+    this.formatter.setup({
+      locale,
+      translations: {
+        [locale]: this.formatter.setup().translations[locale] ?? {},
+      },
+    });
   }
 
   /**
    * Returns the formatter's current locale.
    */
-  getLocale() {
-    return this.formatter.locale;
+  getLocale(): string {
+    return (Array.isArray(this.formatter.setup().locale) ? this.formatter.setup().locale[0] : this.formatter.setup().locale) as string;
   }
 
   addTranslations(translations: Translations) {
-    Object.assign(this.translations, translations);
-  }
+    const locale = this.getLocale();
 
-  /**
-   * An extensible entrypoint for extenders to register type handlers for translations.
-   */
-  protected formatterTypeHandlers() {
-    return {
-      plural: pluralTypeHandler,
-      select: selectTypeHandler,
-    };
+    this.formatter.setup({
+      translations: {
+        [locale]: Object.assign(this.translations, translations),
+      },
+    });
   }
 
   /**
    * A temporary system to preprocess parameters.
    * Should not be used by extensions.
-   * TODO: An extender will be added in v1.x.
    *
    * @internal
    */
-  protected preprocessParameters(parameters: TranslatorParameters) {
+  protected preprocessParameters(parameters: TranslatorParameters, translation: string | Translation) {
     // If we've been given a user model as one of the input parameters, then
     // we'll extract the username and use that for the translation. In the
     // future there should be a hook here to inspect the user and change the
@@ -75,23 +77,66 @@ export default class Translator {
       if (!parameters.username) parameters.username = username(user);
     }
 
+    // To maintain backwards compatibility, we will catch HTML elements and
+    // push the tags as mithril children to the parameters keyed by the tag name.
+    // Will be removed in v2.0
+    translation = typeof translation === 'string' ? translation : translation.message;
+    const elements = translation.match(/<(\w+)[^>]*>.*?<\/\1>/g);
+    const tags = elements?.map((element) => element.match(/^<(\w+)/)![1]) || [];
+
+    for (const tag of tags) {
+      if (!parameters[tag]) {
+        fireDebugWarning(
+          `Any HTML tags used within translations must have corresponding mithril component parameters.\nCaught in translation: \n\n"""\n${translation}\n"""`,
+          '',
+          'v2.0',
+          'flarum/framework'
+        );
+
+        parameters[tag] = ({ children }: any) => m(tag, children);
+      }
+    }
+
+    // The old formatter allowed rich parameters as such:
+    // { link: <Link href="https://flarum.org"/> }
+    // The new formatter dictates that the rich parameter must be a function,
+    // like so: { link: ({ children }) => <Link href="https://flarum.org">{children}</Link> }
+    // This layer allows the old format to be used, and converts it to the new format.
+    for (const key in parameters) {
+      const value: any = parameters[key];
+
+      if (tags.includes(key) && typeof value === 'object' && value.attrs && value.tag) {
+        parameters[key] = ({ children }: any) => {
+          return m(value.tag, value.attrs, children);
+        };
+      }
+    }
+
     return parameters;
   }
 
-  trans(id: string, parameters: TranslatorParameters): NestedStringArray;
-  trans(id: string, parameters: TranslatorParameters, extract: false): NestedStringArray;
+  trans(id: string, parameters: TranslatorParameters): any[];
+  trans(id: string, parameters: TranslatorParameters, extract: false): any[];
   trans(id: string, parameters: TranslatorParameters, extract: true): string;
-  trans(id: string): NestedStringArray | string;
+  trans(id: string): any[] | string;
   trans(id: string, parameters: TranslatorParameters = {}, extract = false) {
-    const translation = this.translations[id];
+    const translation = this.preprocessTranslation(this.translations[id]);
 
     if (translation) {
-      parameters = this.preprocessParameters(parameters);
-      const locale = this.formatter.rich(translation, parameters);
+      parameters = this.preprocessParameters(parameters, translation);
+
+      this.translations[id] = translation;
+
+      let locale = this.formatter.rich({ id, default: id }, parameters);
+
+      // convert undefined args to {undefined}.
+      locale = locale instanceof Array ? locale.map((arg) => (arg === undefined ? '{undefined}' : arg)) : locale;
 
       if (extract) return extractText(locale);
 
       return locale;
+    } else {
+      fireDebugWarning(`Missing translation for key: "${id}"`);
     }
 
     return id;
@@ -113,6 +158,32 @@ export default class Translator {
       if (result) return result;
     }
 
-    return time.format(this.translations[id]);
+    return time.format(this.preprocessTranslation(this.translations[id]));
+  }
+
+  /**
+   * Backwards compatibility for translations such as `<a href='{href}'>`, the old
+   * formatter supported that, but the new one doesn't, so attributes are auto dropped
+   * to avoid errors.
+   *
+   * @private
+   */
+  private preprocessTranslation(translation: string | Translation | undefined): string | undefined {
+    if (!translation) return;
+
+    translation = typeof translation === 'string' ? translation : translation.message;
+
+    // If the translation contains a <x ...attrs> tag, then we'll need to
+    // remove the attributes for backwards compatibility. Will be removed in v2.0.
+    // And if it did have attributes, then we'll fire a warning
+    if (translation.match(/<\w+ [^>]+>/g)) {
+      fireDebugWarning(
+        `Any HTML tags used within translations must be simple tags, without attributes.\nCaught in translation: \n\n"""\n${translation}\n"""`
+      );
+
+      return translation.replace(/<(\w+)([^>]*)>/g, '<$1>');
+    }
+
+    return translation;
   }
 }

framework/core/js/src/common/helpers/fireDebugWarning.ts

@@ -12,7 +12,7 @@ import app from '../app';
  * can fix.
  */
 export default function fireDebugWarning(...args: Parameters<typeof console.warn>): void {
-  if (!app.forum.attribute('debug')) return;
+  if (!app.data.resources.find((r) => r.type === 'forums')?.attributes?.debug) return;
 
   console.warn(...args);
 }

framework/core/js/src/common/utils/abbreviateNumber.ts

@@ -9,7 +9,6 @@ import extractText from './extractText';
  * // "1.2K"
  */
 export default function abbreviateNumber(number: number): string {
-  // TODO: translation
   if (number >= 1000000) {
     return Math.floor(number / 1000000) + extractText(app.translator.trans('core.lib.number_suffix.mega_text'));
   } else if (number >= 1000) {

framework/core/js/src/forum/components/AccessTokensList.tsx

@@ -9,7 +9,6 @@ import classList from '../../common/utils/classList';
 import Tooltip from '../../common/components/Tooltip';
 import type Mithril from 'mithril';
 import type AccessToken from '../../common/models/AccessToken';
-import { NestedStringArray } from '@askvortsov/rich-icu-message-formatter';
 import Icon from '../../common/components/Icon';
 
 export interface IAccessTokensListAttrs extends ComponentAttrs {
@@ -187,7 +186,7 @@ export default class AccessTokensList<CustomAttrs extends IAccessTokensListAttrs
     m.redraw();
   }
 
-  generateTokenTitle(token: AccessToken): NestedStringArray {
+  generateTokenTitle(token: AccessToken): any[] | string {
     const name = token.title() || app.translator.trans('core.forum.security.token_title_placeholder');
     const value = this.tokenValueDisplay(token);
 

framework/core/js/tests/unit/common/utils/Translator.test.ts

@@ -0,0 +1,69 @@
+import Translator from '../../../../src/common/Translator';
+import extractText from '../../../../src/common/utils/extractText';
+
+/*
+ * These tests should be in sync with PHP tests in `tests/unit/Locale/TranslatorTest.php`, to make sure that JS
+ * translator works in the same way as JS translator.
+ */
+
+test('placeholders encoding', () => {
+  const translator = new Translator();
+  translator.addTranslations({
+    test1: 'test1 {placeholder} test1',
+    test2: 'test2 {placeholder} test2',
+  });
+
+  expect(extractText(translator.trans('test1', { placeholder: "'" }))).toBe("test1 ' test1");
+  expect(extractText(translator.trans('test1', { placeholder: translator.trans('test2', { placeholder: "'" }) }))).toBe("test1 test2 ' test2 test1");
+});
+
+// This is how the backend translator behaves. The only discrepancy with the frontend translator.
+// test('missing placeholders', () => {
+//   const translator = new Translator();
+//   translator.addTranslations({
+//     test1: 'test1 {placeholder} test1',
+//   });
+//
+//   expect(extractText(translator.trans('test1', {}))).toBe('test1 {placeholder} test1');
+// });
+
+test('missing placeholders', () => {
+  const translator = new Translator();
+  translator.addTranslations({
+    test1: 'test1 {placeholder} test1',
+  });
+
+  expect(extractText(translator.trans('test1', {}))).toBe('test1 {undefined} test1');
+});
+
+test('escaped placeholders', () => {
+  const translator = new Translator();
+  translator.addTranslations({
+    test3: "test1 {placeholder} '{placeholder}' test1",
+  });
+
+  expect(extractText(translator.trans('test3', { placeholder: "'" }))).toBe("test1 ' {placeholder} test1");
+});
+
+test('plural rules', () => {
+  const translator = new Translator();
+  translator.addTranslations({
+    test4: '{pageNumber, plural, =1 {{forumName}} other {Page # - {forumName}}}',
+  });
+
+  expect(extractText(translator.trans('test4', { forumName: 'A & B', pageNumber: 1 }))).toBe('A & B');
+  expect(extractText(translator.trans('test4', { forumName: 'A & B', pageNumber: 2 }))).toBe('Page 2 - A & B');
+});
+
+test('plural rules 2', () => {
+  const translator = new Translator();
+  translator.setLocale('pl');
+  translator.addTranslations({
+    test5: '{count, plural, one {# post} few {# posty} many {# postów} other {# posta}}',
+  });
+
+  expect(extractText(translator.trans('test5', { count: 1 }))).toBe('1 post');
+  expect(extractText(translator.trans('test5', { count: 2 }))).toBe('2 posty');
+  expect(extractText(translator.trans('test5', { count: 5 }))).toBe('5 postów');
+  expect(extractText(translator.trans('test5', { count: 1.5 }))).toBe('1,5 posta');
+});