(@fluent/react) Add useTranslate

[macabeus]

Closes this issue: #467

Add useTranslate custom hook, that uses the FluentContext to return two functions to get the translated messages from Fluent's bundle.

• t: get the root message from the given id
• tAttributes: get the attributes message from the given id

This PR also updates one example file on @fluent/react in order to show how to use that.

[macabeus]

I don't know if adding a function tAttributes is the best way, but I think that it's enough.
At the first moment I tried adding a parameter on t function to return the attributes (t('type-name'), undefined, { attributes: true })), but it's very weird and bad to type.

[macabeus]

I'm returning an empty string and empty object because I'm following the same approach of <Localized />, but maybe would be better to return the id.
On the case of tAttributes, we could use Proxy to return the attribute name. For example:

tAttributes('invalid-key').foo // should return `"foo"`

[stasm]

Thank you for the PR, @macabeus! I've been thinking about adding a hook-based API too, and it's great to see a contribution in this area. I'd like to spend a bit of time discussing the API design and the use-cases, and then I'll be happy to do a code review. CC'ing @Gregoor who re-wrote @fluent/react to function components which use hooks internally.

---

The API I had in mind was a bit different, and I wonder what you think about it. Rather than return t and tAttributes, could we directly return the ReactLocalization instance? It already has the getString method which is similar to your t function. We could then do:

let l10n = useLocalization();
alert(l10n.getString("hello"));

Or even:

let l10n = useContext(FluentContext);
alert(l10n.getString("hello"));

What I like in this approach is that it mostly relies on abstractions already built into React and into @fluent/react. I usually try to look for solutions which avoid adding more code :)

---

I also have a suggestion regarding the tAttributes function. In Fluent, querying individual attributes alone shouldn't be needed. In other words, the most common operation is either querying the value of the message (like in getString), or querying the value and the attributes.

In @fluent/dom we have two functions corresponding to these two use-cases: valueFromBundle (which is functionally equivalent to @fluent/react's getString, and messageFromValue, which is implemented like this:

fluent.js/fluent-dom/src/localization.js

Lines 215 to 235 in cff95f6

	function messageFromBundle(bundle, errors, message, args) {
	const formatted = {
	value: null,
	attributes: null,
	};
	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};
	}
	}
	return formatted;
	}

Perhaps we could add a similar method to ReactLocalization to allow formatting the value and the attributes in one go?

---

That said, I might suggest startig with just getString for now. For more complex usage, I think wrapping components in <Localized> should be the preferred way of localizing them together with attributes.

[macabeus]

Thank you for commenting on the PR. And yeah, I also want to discuss more about the API =]

---

Rather than return t and tAttributes, could we directly return the ReactLocalization instance? It already has the getString method which is similar to your t function.

I didn't notice about ReactLocalization.
I'm making some prototypes following this approach, and yeah, I think that returning a ReactLocalization would be enough.

I like the mindset of creating a new feature with few new line of codes, but:

• I think that the name getString is too big. Is very common to use many times the function that translate a string, and because of that is a common convention to call this function just as t. A smaller name is good on this case. Maybe we could rename getString to t?
• I'm a little afraid to useTranslate just returning ReactLocalization instance instead of a plain object, because it'll would bad if we want to change its return on future to returning something more. Do you think that it could be a problem?

---

Perhaps we could add a similar method to ReactLocalization to allow formatting the value and the attributes in one go?

Yeah, adding a new method that returns everything would be nice. Looks like a better approach following the fluent mindset.

But I think that the name messageFromValue isn't expressive enough. Maybe we could still call as tAttributes?

[stasm]

I think that the name getString is too big. Is very common to use many times the function that translate a string, and because of that is a common convention to call this function just as t. A smaller name is good on this case. Maybe we could rename getString to t?

I think t is a good choice when the function is expected to be used a lot, like in i18next. For @fluent/react, I think we should stick to composing components via <Localized> and only use getString in exceptional scenarios, when the imperative API is the only way to solve a problem. The way I think about the API I'd like to add here is that it's a hook equivalent of the withLocalization HOC, catering to the same use-caes.

That's why I'd suggest to only focus on the getString use-case. For attributes, let's leave that out of this PR, please. We might want to add a method for formatting an entire message, together with its attributes, later on, but for now, I'd like to focus on <Localized> as the work horse.

I'm a little afraid to useTranslate just returning ReactLocalization instance instead of a plain object, because it'll would bad if we want to change its return on future to returning something more. Do you think that it could be a problem?

Hmm, that's an interesting point. I think you're right that it might be safer to return a plain object, like so:

let {l10n} = useLocalization();

In the future, we might want to add more properties, for example:

let {l10n, changeLocales} =  useLocalization();

I'd like to think about this a little bit and see what's possible. Let's not block this PR on this; both approaches will be fine :)

[macabeus]

@stasm Okay. I just changed the PR to useTranslate return an instance of ReactLocalization (and a fallback object if there isn't that, following the null object pattern).

Also I changed the example to just replace withLocalization with the new hook.

[stasm]

Thanks for your patience, @macabeus, a few other things kept me busy ealier this week. I think the PR is close to being ready, I just have a few comments which I'd like to see addressed first. Let me know what you think :)

[macabeus]

@stasm I made the changes that you proposed.
Also I added some very simple tests case for useLocalization. I added just three new tests because I'm avoiding to add redundant tests.

[stasm]

This looks great, thanks! I really like how it ended up being not a lot of code, since we mostly rely on React's Context API. And I like how useLocalization allows us to control our API. Great work!

[stasm]

@macabeus Let me know when you're ready for getting this merged. I'll also update the docs on the wiki once this is released. Thanks again!

[macabeus]

@stasm It's fine to me to merge =]
Thank you