Fix cloneElement when child is null or string

Author: stasm

fluent-react/src/localized.js

@@ -79,29 +79,41 @@ export default class Localized extends Component {
 
   render() {
     const { l10n, parseMarkup } = this.context;
-    const { id, attrs, children: elem = null } = this.props;
+    const { id, attrs, children: child = null } = this.props;
 
     // Validate that the child element isn't an array
-    if (Array.isArray(elem)) {
+    if (Array.isArray(child)) {
       throw new Error("<Localized/> expected to receive a single " +
         "React node child");
     }
 
     if (!l10n) {
       // Use the wrapped component as fallback.
-      return elem;
+      return child;
     }
 
     const bundle = l10n.getBundle(id);
 
     if (bundle === null) {
       // Use the wrapped component as fallback.
-      return elem;
+      return child;
     }
 
     const msg = bundle.getMessage(id);
     const [args, elems] = toArguments(this.props);
 
+    // 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);
+      }
+
+      return child;
+    }
+
     // 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.
@@ -115,34 +127,27 @@ export default class Localized extends Component {
       }
     }
 
+    // If the wrapped component is a known void element, explicitly dismiss the
+    // message value and do not pass it to cloneElement in order to avoid the
+    // "void element tags must neither have `children` nor use
+    // `dangerouslySetInnerHTML`" error.
+    if (child.type in VOID_ELEMENTS) {
+      return cloneElement(child, localizedProps);
+    }
+
     // If the message has a null value, we're only interested in its attributes.
     // Do not pass the null value to cloneElement as it would nuke all children
     // of the wrapped component.
     if (msg.value === null) {
-      return cloneElement(elem, localizedProps);
+      return cloneElement(child, localizedProps);
     }
 
     const messageValue = bundle.formatPattern(msg.value, args);
 
-    // Check if the fallback is a valid element -- if not then it's not
-    // markup (e.g. nothing or a fallback string) so just use the
-    // formatted message value
-    if (!isValidElement(elem)) {
-      return messageValue;
-    }
-
-    // If the wrapped component is a known void element, explicitly dismiss the
-    // message value and do not pass it to cloneElement in order to avoid the
-    // "void element tags must neither have `children` nor use
-    // `dangerouslySetInnerHTML`" error.
-    if (elem.type in VOID_ELEMENTS) {
-      return cloneElement(elem, localizedProps);
-    }
-
     // If the message value doesn't contain any markup nor any HTML entities,
     // insert it as the only child of the wrapped component.
     if (!reMarkup.test(messageValue)) {
-      return cloneElement(elem, localizedProps, messageValue);
+      return cloneElement(child, localizedProps, messageValue);
     }
 
     // If the message contains markup, parse it and try to match the children
@@ -175,7 +180,7 @@ export default class Localized extends Component {
       return cloneElement(sourceChild, null, childNode.textContent);
     });
 
-    return cloneElement(elem, localizedProps, ...translatedChildren);
+    return cloneElement(child, localizedProps, ...translatedChildren);
   }
 }
 

fluent-react/test/localized_render_test.js

@@ -278,6 +278,25 @@ foo = { $arg }
     assert.equal(wrapper.text(), 'String fallback');
   });
 
+  test('render with a string fallback and no message value preserves the fallback',
+  function() {
+    const mcx = new FluentBundle();
+    const l10n = new ReactLocalization([mcx]);
+    mcx.addMessages(`
+foo =
+    .attr = Attribute
+`)
+
+    const wrapper = shallow(
+      <Localized id="foo">
+        String fallback
+      </Localized>,
+      { context: { l10n } }
+    );
+
+    assert.equal(wrapper.text(), 'String fallback');
+  });
+
   test('render with a string fallback returns the message', function() {
     const mcx = new FluentBundle();
     const l10n = new ReactLocalization([mcx]);
@@ -295,32 +314,51 @@ foo = Test message
     assert.equal(wrapper.text(), 'Test message');
   });
 
-  test('render without a fallback returns the message', function() {
+  test('render without a fallback and no message returns nothing',
+  function() {
+    const mcx = new FluentBundle();
+    const l10n = new ReactLocalization([mcx]);
+
+    const wrapper = shallow(
+      <Localized id="foo" />,
+      { context: { l10n } }
+    );
+
+    assert.equal(wrapper.text(), '');
+  });
+
+  test('render without a fallback and no message value returns nothing',
+  function() {
     const mcx = new FluentBundle();
     const l10n = new ReactLocalization([mcx]);
 
     mcx.addMessages(`
-foo = Message
+foo =
+    .attr = Attribute
 `)
 
     const wrapper = shallow(
       <Localized id="foo" />,
       { context: { l10n } }
     );
 
-    assert.equal(wrapper.text(), 'Message');
+    assert.equal(wrapper.text(), '');
   });
 
-  test('render without a fallback and no message returns nothing',
-  function() {
+  test('render without a fallback returns the message', function() {
     const mcx = new FluentBundle();
     const l10n = new ReactLocalization([mcx]);
 
+    mcx.addMessages(`
+foo = Message
+`)
+
     const wrapper = shallow(
       <Localized id="foo" />,
       { context: { l10n } }
     );
 
-    assert.equal(wrapper.text(), '');
+    assert.equal(wrapper.text(), 'Message');
   });
+
 });