fluent-react: Report errors from Localized and getString

Author: stasm

fluent-react/src/localization.js

@@ -58,12 +58,25 @@ export default class ReactLocalization {
     if (bundle) {
       const msg = bundle.getMessage(id);
       if (msg && msg.value) {
-        return bundle.formatPattern(msg.value, args);
+        let errors = [];
+        let value = bundle.formatPattern(msg.value, args, errors);
+        for (let error of errors) {
+          this.reportError(error);
+        }
+        return value;
       }
     }
 
     return fallback || id;
   }
+
+  // XXX Control this via a prop passed to the LocalizationProvider.
+  // See https://github.com/projectfluent/fluent.js/issues/411.
+  reportError(error) {
+    /* global console */
+    // eslint-disable-next-line no-console
+    console.warn(`[@fluent/react] ${error.name}: ${error.message}`);
+  }
 }
 
 export function isReactLocalization(props, propName) {

fluent-react/src/localized.js

@@ -101,30 +101,41 @@ export default class Localized extends Component {
 
     const msg = bundle.getMessage(id);
     const [args, elems] = toArguments(this.props);
+    let errors = [];
 
     // Check if the child inside <Localized> is a valid element -- if not, then
     // it's either null or a simple fallback string. No need to localize the
     // attributes.
     if (!isValidElement(child)) {
       if (msg.value) {
         // Replace the fallback string with the message value;
-        return bundle.formatPattern(msg.value, args);
+        let value = bundle.formatPattern(msg.value, args, errors);
+        for (let error of errors) {
+          l10n.reportError(error);
+        }
+        return value;
       }
 
       return child;
     }
 
+    let localizedProps;
+
     // The default is to forbid all message attributes. If the attrs prop exists
     // on the Localized instance, only set message attributes which have been
     // explicitly allowed by the developer.
     if (attrs && msg.attributes) {
-      var localizedProps = {};
+      localizedProps = {};
+      errors = [];
       for (const [name, allowed] of Object.entries(attrs)) {
         if (allowed && name in msg.attributes) {
           localizedProps[name] = bundle.formatPattern(
-            msg.attributes[name], args);
+            msg.attributes[name], args, errors);
         }
       }
+      for (let error of errors) {
+        l10n.reportError(error);
+      }
     }
 
     // If the wrapped component is a known void element, explicitly dismiss the
@@ -142,7 +153,11 @@ export default class Localized extends Component {
       return cloneElement(child, localizedProps);
     }
 
-    const messageValue = bundle.formatPattern(msg.value, args);
+    errors = [];
+    const messageValue = bundle.formatPattern(msg.value, args, errors);
+    for (let error of errors) {
+      l10n.reportError(error);
+    }
 
     // If the message value doesn't contain any markup nor any HTML entities,
     // insert it as the only child of the wrapped component.

fluent-react/test/localized_render_test.js

@@ -223,7 +223,7 @@ foo =
 
 
   test('$arg is passed to format the value', function() {
-    const bundle = new FluentBundle();
+    const bundle = new FluentBundle("en", {useIsolating: false});
     const formatPattern = sinon.spy(bundle, 'formatPattern');
     const l10n = new ReactLocalization([bundle]);
 
@@ -240,6 +240,10 @@ foo = { $arg }
 
     const { args } = formatPattern.getCall(0);
     assert.deepEqual(args[1], { arg: 'ARG' });
+
+    assert.ok(wrapper.contains(
+      <div>ARG</div>
+    ));
   });
 
   test('$arg is passed to format the attributes', function() {
@@ -249,18 +253,45 @@ foo = { $arg }
 
     bundle.addResource(new FluentResource(`
 foo = { $arg }
-    .attr = { $arg }
+    .title = { $arg }
 `));
 
     const wrapper = shallow(
-      <Localized id="foo" $arg="ARG">
+      <Localized id="foo" attrs={{title: true}} $arg="ARG">
         <div />
       </Localized>,
       { context: { l10n } }
     );
 
-    const { args } = formatPattern.getCall(0);
-    assert.deepEqual(args[1], { arg: 'ARG' });
+    // The value.
+    assert.deepEqual(formatPattern.getCall(0).args[1], { arg: 'ARG' });
+    // The attribute.
+    assert.deepEqual(formatPattern.getCall(1).args[1], { arg: 'ARG' });
+
+    assert.ok(wrapper.contains(
+      <div title="ARG">ARG</div>
+    ));
+  });
+
+  test('A missing $arg does not break rendering', function() {
+    const bundle = new FluentBundle("en", {useIsolating: false});
+    const l10n = new ReactLocalization([bundle]);
+
+    bundle.addResource(new FluentResource(`
+foo = { $arg }
+    .title = { $arg }
+`));
+
+    const wrapper = shallow(
+      <Localized id="foo" attrs={{title: true}}>
+        <div />
+      </Localized>,
+      { context: { l10n } }
+    );
+
+    assert.ok(wrapper.contains(
+      <div title="{$arg}">{"{$arg}"}</div>
+    ));
   });
 
   test('render with a fragment and no message preserves the fragment',

fluent-react/test/with_localization_test.js

@@ -31,12 +31,13 @@ suite('withLocalization', function() {
   });
 
   test('getString with access to the l10n context', function() {
-    const bundle = new FluentBundle();
+    const bundle = new FluentBundle("en", {useIsolating: false});
     const l10n = new ReactLocalization([bundle]);
     const EnhancedComponent = withLocalization(DummyComponent);
 
     bundle.addResource(new FluentResource(`
 foo = FOO
+bar = BAR {$arg}
 `));
 
     const wrapper = shallow(
@@ -47,15 +48,19 @@ foo = FOO
     const getString = wrapper.prop('getString');
     // Returns the translation.
     assert.strictEqual(getString('foo', {}), 'FOO');
+    assert.strictEqual(getString('bar', {arg: 'ARG'}), 'BAR ARG');
+    // Doesn't throw on formatting errors.
+    assert.strictEqual(getString('bar', {}), 'BAR {$arg}');
   });
 
   test('getString with access to the l10n context, with fallback value', function() {
-    const bundle = new FluentBundle();
+    const bundle = new FluentBundle("en", {useIsolating: false});
     const l10n = new ReactLocalization([bundle]);
     const EnhancedComponent = withLocalization(DummyComponent);
 
     bundle.addResource(new FluentResource(`
 foo = FOO
+bar = BAR {$arg}
 `));
 
     const wrapper = shallow(
@@ -65,7 +70,12 @@ foo = FOO
 
     const getString = wrapper.prop('getString');
     // Returns the translation, even if fallback value provided.
-    assert.strictEqual(getString('bar', {}, 'fallback'), 'fallback');
+    assert.strictEqual(getString('foo', {}, 'fallback'), 'FOO');
+    // Returns the fallback.
+    assert.strictEqual(getString('missing', {}, 'fallback'), 'fallback');
+    assert.strictEqual(getString('bar', {arg: 'ARG'}), 'BAR ARG');
+    // Doesn't throw on formatting errors.
+    assert.strictEqual(getString('bar', {}), 'BAR {$arg}');
   });
 
   test('getString without access to the l10n context', function() {