Skip to content
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

i18n: add support for multiple instances and reactivity to the i18n library #19210

Closed
jsnajdr opened this issue Dec 18, 2019 · 10 comments
Closed
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Package] i18n /packages/i18n [Status] In Progress Tracking issues with work in progress [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@jsnajdr
Copy link
Member

jsnajdr commented Dec 18, 2019

The @wordpress/i18n library is currently a very simple module (60 lines of code when comments are ignored) that acts as a singleton with essentially two methods:

  • setLocaleData( data ) stores the supplied translation data in an internal global variable
  • __( text ) looks up a text in the global variable and returns a translation from there

In this issue, I'm arguing that this design is insufficient for many WordPress-related use cases, and proposing several enhancements, all of them 100% backwards compatible.

Problem 1: multiple instances and server-side rendering
Consider building a Node.js server with Express.js that used React to render a localized login form. The code would look like this:

import { setLocaleData, __ } from '@wordpress/i18n';

function LoginForm() {
  return (
    <form>
      <label>{ __( 'Username:' ) }</label>
      <input name="username" type="text"></input>
      <label>{ __( 'Password:' ) }</label>
      <input name="password" type="password"></input>
      <button type="submit">{ __( 'Login' ) }</button>
    </form>
  );
}

app.get( '/login/:lang', async ( req, res ) => {
  const langData = await readJSONFile( `./locales/${ req.params.lang }.json` );
  setLocaleData( langData );
  res.send( ReactDOMServer.renderToString( <LoginForm /> ) );
} );

When handling a route like /login/de, the request handler reads a localization data file for German, loads them into the @wordpress/i18n library, and then render the <LoginForm> component into string and send the string in the HTTP response. The __() calls inside the <LoginForm> component will use the data for German.

This kind of code has one very obvious shortcoming: when handling two requests in parallel, e.g., /login/de and /login/es, both handlers will share one instance of @wordpress/i18n and will be overwriting each other's data. The language of the rendered login page will be somewhat random.

At the same time, with the current @wordpress/i18n, we can't do any better.

For Node.js apps that use server-side rendering, for example, rich admin apps like Calypso or headless WordPress frontends like Frontity, using @wordpress/components and @wordpress/i18n to create UIs is not a reliable option.

The solution I'm proposing has two parts:

  1. Support creating multiple independent instances of a I18N class:
import { I18N } from '@wordpress/i18n';

const deLocale = new I18N();
deLocale.setLanguageData( deData );

const nlLocale = new I18N();
nlLocale = nlLocale.setLanguageData( nlData );

deLocale.__( 'Password' ); // Passwort
nlLocale.__( 'Password' ); // Wachtwoord
  1. Pass the I18N instance to a React tree in context:
const I18NContext = React.createContext();

const LoginFormDE = (
  <I18NContext.Provider value={ deLocale } >
    <LoginForm />
  </I18NContext.Provider>
);

const LoginFormNL = (
  <I18NContext.Provider value={ nlLocale } >
    <LoginForm />
  </I18NContext.Provider>
);

ReactDOMServer.renderToString( <LoginFormDE /> );
ReactDOMServer.renderToString( <LoginFormNL /> );

This renders two versions of the login form, each with independent locale data and not influencing each other, given that the LoginForm component uses the translation functions from context:

function LoginForm() {
  const { __ } = useContext( I18NContext );

  return (
    <form>
      <label>{ __( 'Username:' ) }</label>
      <input name="username" type="text"></input>
      <label>{ __( 'Password:' ) }</label>
      <input name="password" type="password"></input>
      <button type="submit">{ __( 'Login' ) }</button>
    </form>
  );
}

Note that we had to add just one line (the useContext hook) to the component to make it use __() from the context rather than from the global.

Using this new capabilities, our Express.js can easily be fixed.

Problem 2: reactive translations
Using the React context for passing the translation functions and data is useful also for other use cases outside server-side rendering. Unlike the original @wordpress/i18n, all localized React components will be automatically rerendered when the context changes. That can be used to implement features like:

  1. Runtime language switcher: open a dropdown in the corner of your WordPress UI, switch locale and boom, the whole UI is immediately rerendered without reloading. Calypso, the WordPress.com React frontend, has this feature and it's useful for supporting users.
  2. Community translator: if you see a missing or incorrect translation in your WordPress UI, open a community translator UI, enter a new translation, press Enter, and you immediately see it applied in your UI. The translator code will update the locale data in the I18N instance and they will trigger a React rerender.

Backward compatibility:
The @wordpress/i18n library can provide a default instance of I18N and export its method as functions:

const defaultI18n = new I18N();
export const setLocaleData = defaultI18n.setLocaleData.bind( defaultI18n );
export const __ = defaultI18n.__.bind( defaultI18n );

and the i18n React context can use this instance as its default value, too:

const I18NContext = React.createContext( defaultI18n );

Then, when a React component uses the const { __ } = useContext( I18NContext ) hook, and there is no provider in the JSX tree, it won't crash -- it will use the default instance.

That provides 100% backward API compatibility without any breaking changes. And lets us update @wordpress/components to use context instead of a global to get the __ function, without breaking any existing usages.

Prior art:
Popular community libraries like react-intl or @lingui/react use the same patterns as described here: core object instances, React context and reactive helpers. See also this recent big overview article about React i18n libraries.

i18n-calypso used in Calypso provides reactive helpers (localize HOC and useTranslate hook), can have multiple instances and can be extended to use React context.

@jsnajdr jsnajdr added [Package] i18n /packages/i18n Internationalization (i18n) Issues or PRs related to internationalization efforts labels Dec 18, 2019
@gziolo gziolo added the Needs Technical Feedback Needs testing from a developer perspective. label Dec 18, 2019
@aduth
Copy link
Member

aduth commented Jan 23, 2020

Runtime language switcher: open a dropdown in the corner of your WordPress UI, switch locale and boom, the whole UI is immediately rerendered without reloading. Calypso, the WordPress.com React frontend, has this feature and it's useful for supporting users.

I considered this early on, though at the time it felt to me the value wasn't very high in contrast with the overhead required to be able to achieve this (multiple, separate APIs when wanting or not wanting reactive translations, higher-order components, etc). A hooks-based solution might be a little more seamless these days.

To me, on how it applies to the real-world: How often are we expecting this locale data to change? For most users, I expect it's assigned once and never changes again. In that light, the current solution seemed acceptable enough in simplifying the developer experience, even if at the expense of reactivity.

Aside: Doesn't switching language on WordPress.com trigger a full-page reload? So even if it's technically possible for those strings to be reactively updated, it doesn't seem to be in use. Has there been much concern over this?

Overall though, I think it could make sense to offer options like the ones you suggestion, in addition to the current defaults.

One thing I'm not sure of is where it would make sense for the React-specific implementation to exist in code. Currently, @wordpress/i18n is concerned only with producing strings, and is otherwise agnostic about how those strings are used. Perhaps it makes sense to have a separate package for the React-specific implementation. Alternatively, maybe it's something where those utilities are bundled in the package, but must be opted into (something like unistore with its unistore/preact and unistore/react bindings).

On the specific point of instances: It could be argued to be something where the use-case is specific enough to warrant using the underlying Tannin library instead, which does support this, and is generally less opinionated.

@aduth aduth removed the Needs Technical Feedback Needs testing from a developer perspective. label Jan 23, 2020
@jsnajdr
Copy link
Member Author

jsnajdr commented Feb 6, 2020

How often are we expecting this locale data to change? For most users, I expect it's assigned once and never changes again. In that light, the current solution seemed acceptable enough

I think the strongest use case here is server-side rendering, where the current solution is simply unusable. The @wordpress/i18n and @wordpress/components packages have an ambition to be a UI foundation for the whole WordPress world, and should eventually work correctly in all major environments.

Doesn't switching language on WordPress.com trigger a full-page reload?

When you change the language in your account setting, then yes, Calypso will be reloaded. But this could be removed today if we wanted. The reasons for doing it mostly disappeared over the last year or two.

Calypso has two, rather minor but still neat, features that change translations at runtime without reload:

  • the Quick Language Switcher that's active in Support Session mode
  • Community Translator that applies translations to the UI immediately after you type them

These couldn't work without reactive translations.

One thing I'm not sure of is where it would make sense for the React-specific implementation to exist in code.

That's a good point. I think the React wrappers should be in a separate package: @wordpress/react-i18n. It's a common design practice to separate a library (like redux) from its React bindings (like react-redux). There are many examples of that.

A hooks-based solution might be a little more seamless these days.

Yes, a useTranslate hook makes the reactive localization much easier and ergonomic. Without the hassle of wrapping everything in a localize HOC.

It could be argued to be something where the use-case is specific enough to warrant using the underlying Tannin library instead

I think that Tannin is a low-level library and that developers don't want to have dcnpgettext() calls in their application code. The __() shorthands are much more convenient. And there's also the well-developed tooling that extracts the __-translated strings from sources and works with them further.

@simison
Copy link
Member

simison commented Feb 7, 2020

[...] the React wrappers should be in a separate package: @wordpress/react-i18n. It's a common design practice to separate a library (like redux) from its React bindings (like react-redux). There are many examples of that.

Just to add another i18n related example: https://www.i18next.com/overview/supported-frameworks

@sirreal
Copy link
Member

sirreal commented Feb 26, 2020

I've been working on React bindings which I'd be happy to contribute to Gutenberg:

Automattic/wp-calypso#39624

@sirreal sirreal mentioned this issue Feb 26, 2020
6 tasks
@aduth
Copy link
Member

aduth commented Feb 27, 2020

One thing I'm not sure of is where it would make sense for the React-specific implementation to exist in code.

That's a good point. I think the React wrappers should be in a separate package: @wordpress/react-i18n. It's a common design practice to separate a library (like redux) from its React bindings (like react-redux). There are many examples of that.

Related to this, if we had a package like this, it seems like it could also be a more appropriate home for __experimentalCreateInterpolateElement, which is currently implemented in @wordpress/element.

cc @nerrad

@nerrad
Copy link
Contributor

nerrad commented Feb 27, 2020

Related to this, if we had a package like this, it seems like it could also be a more appropriate home for __experimentalCreateInterpolateElement, which is currently implemented in @wordpress/element

On the points made in the thread:

  • agreed, new package for react wrappers - I like @wordpress/react-i18n
  • while __experimentalCreateInterpolateElement could be consumed by @wordpress/react-i18n it's not specifically an i18n function. It just happens that's the best use case for it. So I'm not sure it has to live in a i18n package.

@aduth
Copy link
Member

aduth commented Feb 27, 2020

  • while __experimentalCreateInterpolateElement could be consumed by @wordpress/react-i18n it's not specifically a i18n function. It just happens that's the best use case for it. So I'm not sure it has to live in a i18n package.

That's a good point. With that in mind, I don't feel too strongly one way or the other in whether it live in @wordpress/element or some hypothetical React i18n package.

@sirreal
Copy link
Member

sirreal commented Mar 2, 2020

__experimentalCreateInterpolateElement [is] not specifically an i18n function

It's very similar to sprintf in that regard, where sprintf deals with strings and __experimentalCreateInterpolateElement deals with React elements.

What are the use cases for element interpolation outside an i18n context?

@nerrad
Copy link
Contributor

nerrad commented Mar 2, 2020

What are the use cases for element interpolation outside an i18n context?

Any string interpolation that doesn't require localization (which admittedly in the WP context would be suspect). One particular use-case might be some sort of template interpolation?

I've personally always thought it odd that sprintf was in the wp.i18n package too being it doesn't explicitly have a dependency on i18n (it's just a formatter utility). Likely, it's inclusion was just a carryover from it being a part of jed when we were using that).

Regardless, my preference isn't a hill to die on. If folks want this in a @wordpress/react-i18n package I'm okay with that (where it lands shouldn't really matter from a bundle size perspective).

@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Mar 26, 2020
@sirreal sirreal added the [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. label Mar 27, 2020
@sirreal sirreal removed their assignment Mar 27, 2020
@sirreal
Copy link
Member

sirreal commented Jun 25, 2021

With #28465 merged, I believe this is complete.

Migration of existing usage to the new @wordpress/react-i18n library is out of scope.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internationalization (i18n) Issues or PRs related to internationalization efforts [Package] i18n /packages/i18n [Status] In Progress Tracking issues with work in progress [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Development

No branches or pull requests

6 participants