Merge pull request #380 from stasm/formatPattern

FluentBundle.formatPattern

Author: stasm

fluent-dom/src/localization.js

@@ -71,9 +71,9 @@ export default class Localization {
   /**
    * Format translations into {value, attributes} objects.
    *
-   * The fallback logic is the same as in `formatValues` but the argument type
-   * is stricter (an array of arrays) and it returns {value, attributes}
-   * objects which are suitable for the translation of DOM elements.
+   * The fallback logic is the same as in `formatValues` but it returns {value,
+   * attributes} objects which are suitable for the translation of DOM
+   * elements.
    *
    *     docL10n.formatMessages([
    *       {id: 'hello', args: { who: 'Mary' }},
@@ -101,8 +101,8 @@ export default class Localization {
   /**
    * Retrieve translations corresponding to the passed keys.
    *
-   * A generalized version of `DOMLocalization.formatValue`. Keys can
-   * either be simple string identifiers or `[id, args]` arrays.
+   * A generalized version of `DOMLocalization.formatValue`. Keys must
+   * be `{id, args}` objects.
    *
    *     docL10n.formatValues([
    *       {id: 'hello', args: { who: 'Mary' }},
@@ -164,26 +164,26 @@ export default class Localization {
 }
 
 /**
- * Format the value of a message into a string.
+ * Format the value of a message into a string or `null`.
  *
  * This function is passed as a method to `keysFromBundle` and resolve
  * a value of a single L10n Entity using provided `FluentBundle`.
  *
- * If the function fails to retrieve the entity, it will return an ID of it.
- * If formatting fails, it will return a partially resolved entity.
- *
- * In both cases, an error is being added to the errors array.
+ * If the message doesn't have a value, return `null`.
  *
  * @param   {FluentBundle} bundle
- * @param   {Array<Error>}   errors
- * @param   {string}         id
- * @param   {Object}         args
- * @returns {string}
+ * @param   {Array<Error>} errors
+ * @param   {Object} message
+ * @param   {Object} args
+ * @returns {string|null}
  * @private
  */
-function valueFromBundle(bundle, errors, id, args) {
-  const msg = bundle.getMessage(id);
-  return bundle.format(msg, args, errors);
+function valueFromBundle(bundle, errors, message, args) {
+  if (message.value) {
+    return bundle.formatPattern(message.value, args, errors);
+  }
+
+  return null;
 }
 
 /**
@@ -195,34 +195,29 @@ function valueFromBundle(bundle, errors, id, args) {
  * The function will return an object with a value and attributes of the
  * entity.
  *
- * If the function fails to retrieve the entity, the value is set to the ID of
- * an entity, and attributes to `null`. If formatting fails, it will return
- * a partially resolved value and attributes.
- *
- * In both cases, an error is being added to the errors array.
- *
  * @param   {FluentBundle} bundle
- * @param   {Array<Error>}   errors
- * @param   {String}         id
- * @param   {Object}         args
+ * @param   {Array<Error>} errors
+ * @param   {Object} message
+ * @param   {Object} args
  * @returns {Object}
  * @private
  */
-function messageFromBundle(bundle, errors, id, args) {
-  const msg = bundle.getMessage(id);
-
+function messageFromBundle(bundle, errors, message, args) {
   const formatted = {
-    value: bundle.format(msg, args, errors),
+    value: null,
     attributes: null,
   };
 
-  if (msg.attrs) {
-    formatted.attributes = [];
-    for (const [name, attr] of Object.entries(msg.attrs)) {
-      const value = bundle.format(attr, args, errors);
-      if (value !== null) {
-        formatted.attributes.push({name, value});
-      }
+  if (message.value) {
+    formatted.value = bundle.formatPattern(message.value, args, errors);
+  }
+
+  let attrNames = Object.keys(message.attributes);
+  if (attrNames.length > 0) {
+    formatted.attributes = new Array(attrNames.length);
+    for (let [i, name] of attrNames.entries()) {
+      let value = bundle.formatPattern(message.attributes[name], args, errors);
+      formatted.attributes[i] = {name, value};
     }
   }
 
@@ -270,9 +265,10 @@ function keysFromBundle(method, bundle, keys, translations) {
       return;
     }
 
-    if (bundle.hasMessage(id)) {
+    let message = bundle.getMessage(id);
+    if (message) {
       messageErrors.length = 0;
-      translations[i] = method(bundle, messageErrors, id, args);
+      translations[i] = method(bundle, messageErrors, message, args);
       // XXX: Report resolver errors
     } else {
       missingIds.add(id);

fluent-react/src/localization.js

@@ -55,13 +55,14 @@ export default class ReactLocalization {
    */
   getString(id, args, fallback) {
     const bundle = this.getBundle(id);
-
-    if (bundle === null) {
-      return fallback || id;
+    if (bundle) {
+      const msg = bundle.getMessage(id);
+      if (msg && msg.value) {
+        return bundle.formatPattern(msg.value, args);
+      }
     }
 
-    const msg = bundle.getMessage(id);
-    return bundle.format(msg, args);
+    return fallback || id;
   }
 }
 

fluent-react/src/localized.js

@@ -79,45 +79,50 @@ 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);
-    const messageValue = bundle.format(msg, 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;
+    // 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.
-    if (attrs && msg.attrs) {
+    if (attrs && msg.attributes) {
       var localizedProps = {};
       for (const [name, allowed] of Object.entries(attrs)) {
-        if (allowed && msg.attrs.hasOwnProperty(name)) {
-          localizedProps[name] = bundle.format(msg.attrs[name], args);
+        if (allowed && name in msg.attributes) {
+          localizedProps[name] = bundle.formatPattern(
+            msg.attributes[name], args);
         }
       }
     }
@@ -126,21 +131,23 @@ export default class Localized extends Component {
     // 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 (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 (messageValue === null) {
-      return cloneElement(elem, localizedProps);
+    if (msg.value === null) {
+      return cloneElement(child, localizedProps);
     }
 
+    const messageValue = bundle.formatPattern(msg.value, args);
+
     // 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
@@ -173,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

@@ -224,7 +224,7 @@ foo =
 
   test('$arg is passed to format the value', function() {
     const bundle = new FluentBundle();
-    const format = sinon.spy(bundle, 'format');
+    const formatPattern = sinon.spy(bundle, 'formatPattern');
     const l10n = new ReactLocalization([bundle]);
 
     bundle.addMessages(`
@@ -238,13 +238,13 @@ foo = { $arg }
       { context: { l10n } }
     );
 
-    const { args } = format.getCall(0);
+    const { args } = formatPattern.getCall(0);
     assert.deepEqual(args[1], { arg: 'ARG' });
   });
 
   test('$arg is passed to format the attributes', function() {
     const bundle = new FluentBundle();
-    const format = sinon.spy(bundle, 'format');
+    const formatPattern = sinon.spy(bundle, 'formatPattern');
     const l10n = new ReactLocalization([bundle]);
 
     bundle.addMessages(`
@@ -259,7 +259,7 @@ foo = { $arg }
       { context: { l10n } }
     );
 
-    const { args } = format.getCall(0);
+    const { args } = formatPattern.getCall(0);
     assert.deepEqual(args[1], { arg: 'ARG' });
   });
 
@@ -406,6 +406,25 @@ foo = Test message
     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]);
@@ -423,32 +442,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');
   });
+
 });

fluent/README.md

@@ -34,8 +34,10 @@ if (errors.length) {
 
 const welcome = bundle.getMessage('welcome');
 
-bundle.format(welcome, { name: 'Anna' });
-// → 'Welcome, Anna, to Foo 3000!'
+if (welcome.value) {
+    bundle.formatPattern(welcome.value, { name: 'Anna' });
+    // → 'Welcome, Anna, to Foo 3000!'
+}
 ```
 
 The API reference is available at http://projectfluent.org/fluent.js/fluent.