Split Localized into LocalizedElement, LocalizedText

[stasm]

Fix #498.

[stasm]

The PR is not ready for review yet. I need to write tests for the new LocalizedElement and LocalizedText components and I think writing them will require some refactoring of the current tests, unfortunately. I'm also not 100% sure I got all the types right.

Summary: in React, props.children can be any of the following:

1. null or undefined,
2. a string, number or a boolean,
3. a React element, including a React fragment,
4. an array of React elements.

• LocalizedElement supports (3) above.
• LocalizedText supports (1) and the string type from (2).

I'm not sure how much validation these new components should perform. For example, should LocalizedText throw if passed an element child? Or multiple element children? Or, should it replace them with the translation text?

@elisehein What do you think?

[elisehein]

It would make sense to me if correct use of the two components is enforced. Thinking back to your comment here even if it's possible for LocalizedText to handle simple element children, it would break down as soon as you try to use elems, in which case it would be nice to let the user know what the problem is. Perhaps not throw an error, but log a warning so that it still works for the simple use cases?

As an aside, will LocalizedElement support 4. an array of React elements.?

[stasm]

It would make sense to me if correct use of the two components is enforced. Thinking back to your comment here even if it's possible for LocalizedText to handle simple element children, it would break down as soon as you try to use elems, in which case it would be nice to let the user know what the problem is. Perhaps not throw an error, but log a warning so that it still works for the simple use cases?

Good point about validation. It's good to be explicit about the expected input, both from the PoV of the users of @fluent/react and its maintainers. I actually think it's beneficial to throw in case of invalid input. I implemented it in https://github.com/projectfluent/fluent.js/pull/502/commits/75e1c76c17eef41181ac21d15dd0720cddcf8c09.

As an aside, will LocalizedElement support 4. an array of React elements.?

I don't think it should. Since one LocalizedElement corresponds to one Fluent message, I'm not sure what the correct behavior should be if there are more than one child. Am I missing something?

[elisehein]

I don't think it should. Since one LocalizedElement corresponds to one Fluent message, I'm not sure what the correct behavior should be if there are more than one child. Am I missing something?

No, not at all. I'm not sure either, it's a bit of a weird gray area imo... But was just wondering because you wrote it would support React fragments, which could also be a list of multiple children.

What would be the correct Fluent way of translating, say, this bit in your comment above? (genuinely asking, not suggesting)

Summary: in React, props.children can be any of the following:

1. null or undefined,
2. a string, number or a boolean,
3. a React element, including a React fragment,
4. an array of React elements.

It could be seen as a separate introductory phrase followed by four separate short phrases, but I guess it could also be seen as a single sentence that should be translated whole, in one <LocalizedElement>. In the latter case, I'd expect to be able to nest <p> and <ol> as the multiple sibling children of one <LocalizedElement>.

[stasm]

No, not at all. I'm not sure either, it's a bit of a weird gray area imo... But was just wondering because you wrote it would support React fragments, which could also be a list of multiple children.

As far as fragments are concerned, I'd like to support them explicitly via <LocalizedElement><>...</></LocalizedElement>. This has the benefit of still being only a single direct child.

The support for arrays and implicit fragments is something that we could consider in the future, and I think the current API could be extended in a backwards-compatible manner.

It could be seen as a separate introductory phrase followed by four separate short phrases, but I guess it could also be seen as a single sentence that should be translated whole, in one <LocalizedElement>. In the latter case, I'd expect to be able to nest <p> and <ol> as the multiple sibling children of one <LocalizedElement>.

That's a great question! Fluent's primary use-case is localizing the UI, and I'd argue that the example you cited is more content than UI. We've had some ideas about how to bridge the gap between these two different use-cases and how to support larger pieces of text in a single Fluent message. Today, the overlay mechanism (in both @fluent/react and @fluent/dom) only supports flat markup, i.e. no nested elements can appear in the source nor in the translation. There have been experiments centered around lifting this limitation (e.g. https://github.com/zbraniecki/fluent-domoverlays-js/issues/6) but I don't expect us to revisit them in the near future, unfortunately. For now, dividing your example into 5 messages would be the easiest work-around.