Fix i18n types (#33305)

Without the fix the return type would be `string` in cases where it should be `ReactNode`

apps/web/src/languageHandler.tsx

@@ -12,7 +12,7 @@ import _ from "lodash";
 import {
     _t,
     normalizeLanguageKey,
-    type IVariables,
+    type StringVariables,
     KEY_SEPARATOR,
     getLangsJson,
     registerTranslations,
@@ -72,7 +72,7 @@ export class UserFriendlyError extends Error {
 
     public constructor(
         message: TranslationKey,
-        substitutionVariablesAndCause?: Omit<IVariables, keyof ErrorOptions> | ErrorOptions,
+        substitutionVariablesAndCause?: Omit<StringVariables, keyof ErrorOptions> | ErrorOptions,
     ) {
         // Prevent "Could not find /%\(cause\)s/g in x" logs to the console by removing it from the list
         const { cause, ...substitutionVariables } = substitutionVariablesAndCause ?? {};

packages/module-api/src/api/i18n.ts

@@ -33,13 +33,25 @@ export type SubstitutionValue = number | string | ReactNode | ((sub: string) =>
  * Variables to interpolate into a translation.
  * @public
  */
-export type Variables = {
+export type Variables = StringVariables | RichVariables;
-    /**
+
-     * The number of items to count for pluralised translations
+/**
-     */
+ * Variables that are guaranteed to only contain primitive (string-safe) values
+ * @public
+ */
+export interface StringVariables {
+    count?: number;
+    [key: string]: number | string | null | undefined;
+}
+
+/**
+ * Variables that may contain ReactNodes or functions, requiring a ReactNode return
+ * @public
+ */
+export interface RichVariables {
     count?: number;
     [key: string]: SubstitutionValue;
-};
+}
 
 /**
  * Tags to interpolate into a translation, where the value is a ReactNode or a function that returns a ReactNode.
@@ -68,7 +80,7 @@ export interface I18nApi {
      * @param key - The key to translate
      * @param variables - Optional variables to interpolate into the translation
      */
-    translate(this: void, key: keyof Translations, variables?: Variables): string;
+    translate(this: void, key: keyof Translations, variables?: StringVariables): string;
     /**
      * Perform a translation, with optional variables
      * @param key - The key to translate

packages/module-api/src/index.ts

@@ -8,7 +8,15 @@ Please see LICENSE files in the repository root for full details.
 export { ModuleLoader, ModuleIncompatibleError } from "./loader";
 export type { Api, Module, ModuleFactory } from "./api";
 export type { Config, ConfigApi } from "./api/config";
-export type { I18nApi, Variables, Translations, SubstitutionValue, Tags } from "./api/i18n";
+export type {
+    I18nApi,
+    Variables,
+    StringVariables,
+    RichVariables,
+    Translations,
+    SubstitutionValue,
+    Tags,
+} from "./api/i18n";
 export type * from "./models/event";
 export type * from "./models/Room";
 export type * from "./api/composer";

packages/shared-components/src/core/i18n/I18nApi.ts

@@ -8,6 +8,7 @@ Please see LICENSE files in the repository root for full details.
 import {
     type I18nApi as II18nApi,
     type Variables,
+    type StringVariables,
     type Translations,
     type Tags,
 } from "@element-hq/element-web-module-api";
@@ -48,11 +49,11 @@ export class I18nApi implements II18nApi {
      * @param variables - Optional variables to interpolate into the translation
      * @param tags - Optional tags to interpolate into the translation
      */
-    public translate(this: void, key: TranslationKey, variables?: Variables): string;
+    public translate(this: void, key: TranslationKey, variables?: StringVariables): string;
     public translate(this: void, key: TranslationKey, variables: Variables | undefined, tags: Tags): React.ReactNode;
     public translate(this: void, key: TranslationKey, variables?: Variables, tags?: Tags): React.ReactNode | string {
         if (tags) return _t(key, variables, tags);
-        return _t(key, variables);
+        return _t(key, variables as Variables);
     }
 
     public humanizeTime = (timeMillis: number): string => humanizeTime(timeMillis, this);

packages/shared-components/src/core/i18n/i18n.tsx

@@ -141,11 +141,20 @@ function safeCounterpartTranslate(text: string, variables?: IVariables): { trans
  */
 type SubstitutionValue = number | string | React.ReactNode | ((sub: string) => React.ReactNode);
 
-export interface IVariables {
+// Variables that are guaranteed to only contain primitive (string-safe) values
+export interface StringVariables {
+    count?: number;
+    [key: string]: number | string | null | undefined;
+}
+
+// Variables that may contain ReactNodes or functions, requiring a ReactNode return
+export interface RichVariables {
     count?: number;
     [key: string]: SubstitutionValue;
 }
 
+export type IVariables = StringVariables | RichVariables;
+
 export type Tags = Record<string, SubstitutionValue>;
 
 export type TranslatedString = string | React.ReactNode;
@@ -168,13 +177,15 @@ const annotateStrings = (result: TranslatedString, translationKey: TranslationKe
     }
 };
 
-export function _t(text: TranslationKey, variables?: IVariables): string;
+// Returns string only when variables are primitives and no tags are provided
-export function _t(text: TranslationKey, variables: IVariables | undefined, tags: Tags): React.ReactNode;
+export function _t(text: TranslationKey, variables?: StringVariables): string;
-export function _t(text: TranslationKey, variables?: IVariables, tags?: Tags): TranslatedString {
+// Returns ReactNode when variables contain ReactNodes (even without tags)
-    // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution)
+export function _t(text: TranslationKey, variables: RichVariables): React.ReactNode;
+// Returns ReactNode when tags are provided (regardless of variables)
+export function _t(text: TranslationKey, variables: RichVariables | undefined, tags: Tags): React.ReactNode;
+export function _t(text: TranslationKey, variables?: StringVariables | RichVariables, tags?: Tags): TranslatedString {
     const { translated } = safeCounterpartTranslate(text, variables);
     const substituted = substitute(translated, variables, tags);
-
     return annotateStrings(substituted, text);
 }
 
@@ -197,8 +208,9 @@ export function lookupString(key: TranslationKey): string {
  * or translation used a fallback locale, otherwise a string
  */
 // eslint-next-line @typescript-eslint/naming-convention
-export function _tDom(text: TranslationKey, variables?: IVariables): TranslatedString;
+export function _tDom(text: TranslationKey, variables?: StringVariables): string;
-export function _tDom(text: TranslationKey, variables: IVariables, tags: Tags): React.ReactNode;
+export function _tDom(text: TranslationKey, variables: RichVariables): React.ReactNode;
+export function _tDom(text: TranslationKey, variables: RichVariables, tags: Tags): React.ReactNode;
 export function _tDom(text: TranslationKey, variables?: IVariables, tags?: Tags): TranslatedString {
     // The translation returns text so there's no XSS vector here (no unsafe HTML, no code execution)
     const { translated, isFallback } = safeCounterpartTranslate(text, variables);
@@ -233,8 +245,9 @@ export function sanitizeForTranslation(text: string): string {
  *
  * @return a React <span> component if any non-strings were used in substitutions, otherwise a string
  */
-export function substitute(text: string, variables?: IVariables): string;
+export function substitute(text: string, variables?: StringVariables): string;
-export function substitute(text: string, variables: IVariables | undefined, tags: Tags | undefined): string;
+export function substitute(text: string, variables?: RichVariables): React.ReactNode;
+export function substitute(text: string, variables: RichVariables | undefined, tags: Tags | undefined): string;
 export function substitute(text: string, variables?: IVariables, tags?: Tags): string | React.ReactNode {
     let result: React.ReactNode | string = text;