fix(react-intl): reduce onError chattiness

Before when we encounter errors in formatting default message we fire multiple onError which clobbers logging. This makes sure we don't double fire FormatError. Also print out more info about message descriptor.
formatjs · May 18, 2020 · 42d0ac4 · 42d0ac4
1 parent f469e81
commit 42d0ac4
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 89 deletions.
diff --git a/packages/intl-locale/package.json b/packages/intl-locale/package.json
@@ -2,6 +2,7 @@
   "name": "@formatjs/intl-locale",
   "version": "0.0.0",
   "description": "Intl.Locale polyfill",
+  "private": true,
   "keywords": [
     "intl",
     "locale",

diff --git a/packages/intl-locale/tests/index.test.ts b/packages/intl-locale/tests/index.test.ts
@@ -1,7 +1,7 @@
 import {IntlLocale} from '../';
 
 describe('intl-locale', () => {
-  it('needs tests', function() {
+  it('needs tests', function () {
     expect(IntlLocale).toBeTruthy();
   });
 });
diff --git a/packages/intl-locale/tests/runner.ts b/packages/intl-locale/tests/runner.ts
@@ -74,8 +74,9 @@ json.forEach(t => {
 });
 if (failedTests.length) {
   console.log(
-    `Tests: ${failedTests.length} failed, ${json.length -
-      failedTests.length} passed, ${json.length} total`
+    `Tests: ${failedTests.length} failed, ${
+      json.length - failedTests.length
+    } passed, ${json.length} total`
   );
   process.exitCode = 1;
 } else {

diff --git a/packages/react-intl/src/error.ts b/packages/react-intl/src/error.ts
@@ -10,22 +10,50 @@ export const enum ReactIntlErrorCode {
 
 export class ReactIntlError extends Error {
   public readonly code: ReactIntlErrorCode;
+
+  constructor(code: ReactIntlErrorCode, message: string, exception?: Error) {
+    super(
+      `[React Intl Error ${code}] ${message} 
+${exception ? `\n${exception.stack}` : ''}`
+    );
+    this.code = code;
+    if (typeof Error.captureStackTrace === 'function') {
+      Error.captureStackTrace(this, ReactIntlError);
+    }
+  }
+}
+
+export class MessageFormatError extends ReactIntlError {
   public readonly descriptor?: MessageDescriptor;
   constructor(
-    code: ReactIntlErrorCode,
     message: string,
-    descriptor?: MessageDescriptor,
+    locale: string,
+    descriptor: MessageDescriptor,
     exception?: Error
   ) {
     super(
-      `[React Intl Error ${code}] ${message} ${
-        exception ? `\n${exception.stack}` : ''
-      }`
+      ReactIntlErrorCode.FORMAT_ERROR,
+      `${message} 
+Locale: ${locale}
+MessageID: ${descriptor?.id}
+Default Message: ${descriptor?.defaultMessage}
+Description: ${descriptor?.description} 
+`,
+      exception
+    );
+    this.descriptor = descriptor;
+  }
+}
+
+export class MissingTranslationError extends ReactIntlError {
+  public readonly descriptor?: MessageDescriptor;
+  constructor(descriptor: MessageDescriptor, locale: string) {
+    super(
+      ReactIntlErrorCode.MISSING_TRANSLATION,
+      `Missing message: "${descriptor.id}" for locale "${locale}", using ${
+        descriptor.defaultMessage ? 'default message' : 'id'
+      } as fallback.`
     );
-    this.code = code;
     this.descriptor = descriptor;
-    if (typeof Error.captureStackTrace === 'function') {
-      Error.captureStackTrace(this, ReactIntlError);
-    }
   }
 }
diff --git a/packages/react-intl/src/formatters/message.ts b/packages/react-intl/src/formatters/message.ts
@@ -18,7 +18,7 @@ import IntlMessageFormat, {
   FormatXMLElementFn,
   PrimitiveType,
 } from 'intl-messageformat';
-import {ReactIntlError, ReactIntlErrorCode} from '../error';
+import {MessageFormatError, MissingTranslationError} from '../error';
 
 function setTimeZoneInOptions(
   opts: Record<string, Intl.DateTimeFormatOptions>,
@@ -73,9 +73,12 @@ function deepMergeFormatsAndSetTimeZone(
 }
 
 function prepareIntlMessageFormatHtmlOutput(
-  chunks: React.ReactNodeArray
-): React.ReactElement {
-  return React.createElement(React.Fragment, null, ...chunks);
+  chunks: React.ReactNode,
+  shouldWrap?: boolean
+): React.ReactNode {
+  return Array.isArray(chunks) && shouldWrap
+    ? React.createElement(React.Fragment, null, ...chunks)
+    : chunks;
 }
 
 export function formatMessage(
@@ -139,85 +142,88 @@ export function formatMessage(
   formats = deepMergeFormatsAndSetTimeZone(formats, timeZone);
   defaultFormats = deepMergeFormatsAndSetTimeZone(defaultFormats, timeZone);
 
-  let formattedMessageParts: React.ReactNode = '';
-
-  if (message) {
-    try {
-      const formatter = state.getMessageFormat(message, locale, formats, {
-        formatters: state,
-      });
+  if (!message) {
+    if (
+      !defaultMessage ||
+      (locale && locale.toLowerCase() !== defaultLocale.toLowerCase())
+    ) {
+      // This prevents warnings from littering the console in development
+      // when no `messages` are passed into the <IntlProvider> for the
+      // default locale.
+      onError(new MissingTranslationError(messageDescriptor, locale));
+    }
+    if (defaultMessage) {
+      try {
+        const formatter = state.getMessageFormat(
+          defaultMessage,
+          defaultLocale,
+          defaultFormats
+        );
 
-      formattedMessageParts = formatter.format<React.ReactNode>(values);
-    } catch (e) {
-      onError(
-        new ReactIntlError(
-          ReactIntlErrorCode.FORMAT_ERROR,
-          `Error formatting message: "${id}" for locale: "${locale}"` +
-            (defaultMessage ? ', using default message as fallback.' : ''),
-          messageDescriptor,
-          e
-        )
-      );
+        return prepareIntlMessageFormatHtmlOutput(
+          formatter.format(values),
+          wrapRichTextChunksInFragment
+        );
+      } catch (e) {
+        onError(
+          new MessageFormatError(
+            `Error formatting default message for: "${id}", rendering default message verbatim`,
+            locale,
+            messageDescriptor,
+            e
+          )
+        );
+        return defaultMessage;
+      }
     }
-  } else if (
-    !defaultMessage ||
-    (locale && locale.toLowerCase() !== defaultLocale.toLowerCase())
-  ) {
-    // This prevents warnings from littering the console in development
-    // when no `messages` are passed into the <IntlProvider> for the
-    // default locale.
+    return id;
+  }
+
+  // We have the translated message
+  try {
+    const formatter = state.getMessageFormat(message, locale, formats, {
+      formatters: state,
+    });
+
+    return prepareIntlMessageFormatHtmlOutput(
+      formatter.format<React.ReactNode>(values),
+      wrapRichTextChunksInFragment
+    );
+  } catch (e) {
     onError(
-      new ReactIntlError(
-        ReactIntlErrorCode.MISSING_TRANSLATION,
-        `Missing message: "${id}" for locale: "${locale}"` +
-          (defaultMessage ? ', using default message as fallback.' : ''),
-        messageDescriptor
+      new MessageFormatError(
+        `Error formatting message: "${id}", using ${
+          defaultMessage ? 'default message' : 'id'
+        } as fallback.`,
+        locale,
+        messageDescriptor,
+        e
       )
     );
   }
 
-  if (!formattedMessageParts && defaultMessage) {
+  if (defaultMessage) {
     try {
       const formatter = state.getMessageFormat(
         defaultMessage,
         defaultLocale,
         defaultFormats
       );
 
-      formattedMessageParts = formatter.format(values);
+      return prepareIntlMessageFormatHtmlOutput(
+        formatter.format(values),
+        wrapRichTextChunksInFragment
+      );
     } catch (e) {
       onError(
-        new ReactIntlError(
-          ReactIntlErrorCode.FORMAT_ERROR,
-          `Error formatting the default message for: "${id}"`,
+        new MessageFormatError(
+          `Error formatting the default message for: "${id}", rendering message verbatim`,
+          locale,
           messageDescriptor,
           e
         )
       );
     }
   }
-
-  if (!formattedMessageParts) {
-    onError(
-      new ReactIntlError(
-        ReactIntlErrorCode.FORMAT_ERROR,
-        `Cannot format message: "${id}", ` +
-          `using message ${
-            message || defaultMessage ? 'source' : 'id'
-          } as fallback.`,
-        messageDescriptor
-      )
-    );
-    if (typeof message === 'string') {
-      return message || defaultMessage || String(id);
-    }
-    return defaultMessage || String(id);
-  }
-  if (Array.isArray(formattedMessageParts)) {
-    if (wrapRichTextChunksInFragment) {
-      return prepareIntlMessageFormatHtmlOutput(formattedMessageParts);
-    }
-    return formattedMessageParts;
-  }
-  return formattedMessageParts as React.ReactNode;
+  return message || defaultMessage || id;
 }
diff --git a/packages/react-intl/test/unit/__snapshots__/format.tsx.snap b/packages/react-intl/test/unit/__snapshots__/format.tsx.snap
@@ -28,36 +28,31 @@ exports[`format API formatMessage() fallbacks returns \`defaultMessage\` source
 Array [
   "MISSING_TRANSLATION",
   "FORMAT_ERROR",
-  "FORMAT_ERROR",
 ]
 `;
 
 exports[`format API formatMessage() fallbacks returns message \`id\` when message and \`defaultMessage\` are empty 1`] = `
 Array [
   "MISSING_TRANSLATION",
-  "FORMAT_ERROR",
 ]
 `;
 
 exports[`format API formatMessage() fallbacks returns message \`id\` when message and \`defaultMessage\` are missing 1`] = `
 Array [
   "MISSING_TRANSLATION",
-  "FORMAT_ERROR",
 ]
 `;
 
 exports[`format API formatMessage() fallbacks returns message source when formatting error and missing \`defaultMessage\` 1`] = `
 Array [
   "FORMAT_ERROR",
-  "FORMAT_ERROR",
 ]
 `;
 
 exports[`format API formatMessage() fallbacks returns message source when message and \`defaultMessage\` have formatting errors 1`] = `
 Array [
   "FORMAT_ERROR",
   "FORMAT_ERROR",
-  "FORMAT_ERROR",
 ]
 `;
 

diff --git a/packages/react-intl/test/unit/format.tsx b/packages/react-intl/test/unit/format.tsx
@@ -857,11 +857,10 @@ describe('format API', () => {
 
         expect((config.onError as jest.Mock).mock.calls.map(c => c[0].code))
           .toMatchInlineSnapshot(`
-Array [
-  "MISSING_TRANSLATION",
-  "FORMAT_ERROR",
-]
-`);
+          Array [
+            "MISSING_TRANSLATION",
+          ]
+        `);
       });
 
       it('formats `defaultMessage` when message has a syntax error', () => {
@@ -907,12 +906,12 @@ Array [
       });
 
       it('returns message source when message and `defaultMessage` have formatting errors', () => {
-        const {locale, messages} = config;
+        const {messages} = config;
         const id = 'missing_value';
 
         expect(
           formatMessage({
-            id: id,
+            id,
             defaultMessage: messages.invalid,
           })
         ).toBe(messages[id]);
@@ -922,12 +921,12 @@ Array [
       });
 
       it('returns message source when formatting error and missing `defaultMessage`', () => {
-        const {locale, messages} = config;
+        const {messages} = config;
         const id = 'missing_value';
 
         expect(
           formatMessage({
-            id: id,
+            id,
             defaultMessage: messages.missing,
           })
         ).toBe(messages[id]);