Skip to content

FluentBundle.formatPattern #380

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 28 commits into from
Jul 17, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
58dbb91
create-runtime
stasm Apr 24, 2019
77fd38d
FluentBundle.formatPattern
stasm Jun 20, 2019
596151b
Update fixtures
stasm Jun 21, 2019
b55c1cb
Always call toString(scope) in formatPattern.
stasm Jun 21, 2019
e04f35e
Fix resolver tests
stasm Jun 25, 2019
5546c40
Don't fall back on message value when attribute was referenced
stasm Jun 25, 2019
d97ed30
Create a fresh dirty WeakSet in each scope
stasm Jun 25, 2019
018ec95
FluentType.toString(scope)
stasm Jun 25, 2019
01adebc
Minor clean-ups in the resolver
stasm Jun 25, 2019
456be2a
Document formatPattern
stasm Jun 25, 2019
386b9ae
TypeError when resolving null/undefined patterns
stasm Jun 25, 2019
42b2e4d
Rename Type to resolve
stasm Jun 25, 2019
24cdba8
Remove the FluentBundle.messages iterator
stasm Jun 25, 2019
28cce8d
Skip scope creation for simple strings
stasm Jun 26, 2019
148a3c9
Speed up addResource by switching to a C-style for loop
stasm Jun 26, 2019
62e1aad
Fix lint
stasm Jun 26, 2019
16fec82
Merge branch 'master' into formatPattern
stasm Jul 10, 2019
29ffcf3
Update fluent-dom
stasm Jul 10, 2019
dbf45f7
Update fluent-react
stasm Jul 10, 2019
466352e
Separate resolveExpression, resolveComplexPattern
stasm Jul 11, 2019
5cd64fc
Document the special case in resolveExpression
stasm Jul 11, 2019
ef1681a
Improve the docstring for formatPattern
stasm Jul 11, 2019
36df8f7
Use strictEqual when comparing against undefined
stasm Jul 11, 2019
9d27d52
Throw on invalid Pattern type
stasm Jul 11, 2019
729cdb7
Fix assert.throws in node 8.9
stasm Jul 12, 2019
50cb03a
Adjust the comment in FluentType.toString()
stasm Jul 15, 2019
dee06b8
Fix cloneElement when child is null or string
stasm Jul 15, 2019
384dc56
fluent-dom: Fix the attribute count check
stasm Jul 17, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 35 additions & 39 deletions fluent-dom/src/localization.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' }},
Expand Down Expand Up @@ -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' }},
Expand Down Expand Up @@ -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`.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is how it worked before, too. The comment was wrong.

*
* @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;
}

/**
Expand All @@ -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};
}
}

Expand Down Expand Up @@ -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);
Expand Down
11 changes: 6 additions & 5 deletions fluent-react/src/localization.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
45 changes: 26 additions & 19 deletions fluent-react/src/localized.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand All @@ -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
Expand Down Expand Up @@ -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);
}
}

Expand Down
58 changes: 48 additions & 10 deletions fluent-react/test/localized_render_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(`
Expand All @@ -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(`
Expand All @@ -259,7 +259,7 @@ foo = { $arg }
{ context: { l10n } }
);

const { args } = format.getCall(0);
const { args } = formatPattern.getCall(0);
assert.deepEqual(args[1], { arg: 'ARG' });
});

Expand All @@ -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]);
Expand All @@ -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');
});

});
6 changes: 4 additions & 2 deletions fluent/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading