Support non-literal translation keys for i18n

[schnerd]

Type of issue

Feature request

Problem

fusion-plugin-i18n currently only works with string literals:

// Works
<Translate id="homepage.hero" />

// Doesn't work
const key = "homepage.hero";
<Translate id={key} />

This is because there is a custom babel plugin that extracts these strings at build time.

This limitation, however, leads to some very verbose and bloated logic in some cases. For example, when pulling translations for days of the week our old approach:

return Locale.get(`time.weekdays.${day}`);

now turns into the following with Fusion

  switch (dayNumber) {
    case 1:
      return <Translate id="time.weekdays.1" />;
    case 2:
      return <Translate id="time.weekdays.2" />;
    case 3:
      return <Translate id="time.weekdays.3" />;
    case 4:
      return <Translate id="time.weekdays.4" />;
    case 5:
      return <Translate id="time.weekdays.5" />;
    case 6:
      return <Translate id="time.weekdays.6" />;
    case 7:
      return <Translate id="time.weekdays.7" />;
    default:
      return '';
  }

The custom babel transform is awesome and seems necessary for efficient code splitting, but I'm hoping there's a strategy which gives us the best of both worlds.

Potential Solution

One solution that comes to mind is a comment that acts as a directive to the i18n babel transform.

<Translate id={
  // fusion-i18n: time.weekdays.*
  `time.weekdays.${id}`
} />

The transform would parse the comment and match it against keys found in the translation files. A asterisk wildcard match is probably sufficient, but in theory this could also be a regex if people wanted more flexibility.

This solution seems to offer the benefits of code splitting, without runtime overhead and code bloat.

Thoughts?

[schnerd]

cc @rtsao @ganemone

[ganemone]

This is a feature we have been talking about adding for a long time. I don't think there is any downside in theory, its just a bit more complex than it seems so adding it will take some non trivial effort.

[schnerd]

Gotcha. Was the API you all had in mind similar to the one shown above?

its just a bit more complex than it seems so adding it will take some non trivial effort.

Makes sense. If someone wanted to open a PR, what are the key hurdles they'd need to overcome in implementation?

[ganemone]

There is some communication between fusion-cli and fusion-plugin-i18n for managing the translations. I think it is doable, but we should wait for some feedback from @rtsao.

[rtsao]

I think the constraint of key literals still makes sense for <Translate /> but adding a useTranslations hook with support for arbitrary pattern(s) would be ideal for this use case.

I'm imagining something like this:

import {useTranslations} from "fusion-plugin-i18n-react";

function App(props) {
  const [translate] = useTranslations("time.weekdays.*");
  return <div>{translate(`time.weekdays.${props.id}`)}</div>
}

Any translations that match the pattern passed to useTranslations would be preloaded in the containing chunk.

The key hurdle in the implementation is that all translation keys are not known at build time. With key string literals, it is trivial to mark specific translation keys as required. With patterns, these will have to be executed at runtime against the list of discovered translations (probably at server startup?).

I think that modifying the babel plugin and/or core i18n library to do this shouldn't be super difficult, but it does involve a bit more complexity.

[schnerd]

I think the constraint of key literals still makes sense for <Translate /> but adding a useTranslations hook with support for arbitrary pattern(s) would be ideal for this use case.

Out of curiosity – what is the benefit of using a hook for this? Does it need to do things based on the component lifecycle? What will the implementation of useTranslations look like?

Our team is approaching hooks with a healthy dose of skepticism (i.e. they are useful in some cases, but classes are still preferable in other cases). Since hooks aren't supported class components, it would be great to have a non-hooks solution too.

I'm also trying to imagine how this would work when a component needs to translate a lot of different strings:

function App(props) {
  const [translate] = useTranslations("weekdays.*", "time_slots.*", "date_header", "date_subtitle", /* etc... */);
  return (
    <div>
      <h2>{translate('date_header')}</h2>
      <h4>{translate('date_subtitle')}</h4>
      <p>{translate(`weekdays.${props.day}`)} {translate(`time_slots.${props.slot}`)}
    </div>
  );

Seems like you either need to redundantly pass strings to useTranslation or mix the hooks API and <Translate/> API, both of which feel less than ideal.

[rtsao]

Out of curiosity – what is the benefit of using a hook for this? Does it need to do things based on the component lifecycle?

There's not an immediate benefit, it's more about being future-proof. When React fully supports Suspense, a Hook/Component/HOC API will enable a much simpler loading mechanism based on Suspense, giving us the ability to have even more granular translation loading and potentially eliminating the requirement for build steps entirely.

Seems like you either need to redundantly pass strings to useTranslation or mix the hooks API and <Translate/> API, both of which feel less than ideal.

I'm actually inclined to say mixing <Translate /> and useTranslations would be perfectly fine; in fact, I think this is perhaps a perfect illustration of the elegance of hooks in comparison to HOCs or render props (which would be much more cumbersome).

function App(props) {
  const [translate] = useTranslations(["weekdays.*", "time_slots.*"]);
  return (
    <div>
      <h2><Translate id="date_header" /></h2>
      <h4><Translate id="date_subtitle" /></h4>
      <p>{translate(`weekdays.${props.day}`)} {translate(`time_slots.${props.slot}`)}
    </div>
  );

I don't think that this is that bad. In my opinion, it sort of makes sense that useTranslations would be for "dynamic" translations and <Translate /> is for "static" translations.

That said, there is another alternative that we haven't yet discussed:

// Super static analysis
function App(props) {
  const [translate] = useTranslations();
  return (
    <div>
      <h2>{translate("date_header")}</h2>
      <h4>{translate("date_subtitle")}</h4>
      <p>{translate(`weekdays.${props.day}`)} {translate(`time_slots.${props.slot}`)}
    </div>
  );

This is by far the simplest API, but involves more advanced static analysis, additional caveats, and potential footguns.

Taking the exact above example, it is statically possible to determine the following translation key patterns, based on the (template) string literals passed to translate:

• "date_header" (trivial)
• "date_subtitle" (trivial)
• "weekdays.*" from `weekdays.${EXPRESSION}`
• "time_slots.*" from `time_slots.${EXPRESSION}`

This involves static analysis somewhat similar to how "dynamic" import() and require() works in webpack.

This is a pretty elegant solution but does involve a large potential footgun is the following scenario:

function App(props) {
  const [translate] = useTranslations();
  return <div>{translate(props.foo)}</div>;
}

There's nothing here to statically infer about the nature of the translation key. I think there are two reasonable ways of handling this:

1. Include all translations (huge footgun)
2. Throw an error at build-time, requiring that the parameter to translate is string literal or template string that has some prefix.

I'm not sure I like either option. I think the first option would be sensible with either some lint rule or an explicit option passed to useTranslations() that would signal opt-in.

So either this:

function App(props) {
  const [translate] = useTranslations();
  // eslint-disable-next-line fusion-plugin-all-translations
  return <div>{translate(props.foo)}</div>;
}

or

function App(props) {
  const [translate] = useTranslations({includeAll: true});
  return <div>{translate(props.foo)}</div>;
}

It is probably worth noting that we'd necessarily have an additional constraint disallowing usage of the translate function in anything but call expressions. For example, the following would yield a build-time error:

function App(props) {
  const [translate] = useTranslations();
  // Once bound, translate can only be used in call expressions.
  const renamed = translate; // Error, no rebinding allowed
  const result = someHelper(translate); // Error, no passing as an argument
  return <div>{renamed("foo")}</div>;
}

This ensure the static analysis isn't defeated.

---

Of course, hooks are not strictly required. But I would definitely have concerns regarding a comment-based API. I think if there's a scenario where removing a comment ends up breaking your app, this is would be quite counterintuitive and confusing. So for that reason, I'm generally opposed to comment-based APIs, as comments should not, in theory, affect code at runtime at all.

So barring some comment annotation, there needs to be a way to indicate certain translations should be preloaded. This could have many different APIs, but perhaps the simplest being:

import { preloadKeys } from "fusion-plugin-i18n";

preloadKeys(["foo.*", "bar.*"]);

function App({ foo, bar }) {
  return (
    <div>
      <Translate id={`foo.${props.foo}`} />
      <Translate id={`bar.${props.bar}`} />
    </div>
  );
}

I think something like this would also be acceptable, but I think less preferable to the hook-based approaches suggested above.

[schnerd]

Appreciate the thorough reply.

Agree that hooks can provide a better devexp compared to HOCs/render props in many cases. There's still significant disagreement among engineers I've talked to about if and when they should be used, so just wanted to raise a flag before we rush into a hooks-only API. I actually quite like the preloadKeys alternative you shared (and agree that comment-based APIs aren't ideal), so maybe supporting that alongside a hooks API is a solid path forward.

Regarding the alternate const [translate] = useTranslations(); API you shared, I agree that the user should be warned somehow. For what its worth, I actually wouldn't mind a failing build here personally.

[miketikh]

Just wanted to add support to this request for withTranslations as well.

I detailed our team's issue here. Basically, a simple use case like check if a user's search input matches any translated keys becomes more verbose and error-prone when you have to use string literals for the keys.

Looking forward to a potential solution