translation context: Eliminate key hack (the last one!).

In putting together a recent commit, I realized that this would be
possible if our `TranslationContextTranslator` more carefully spoke
the language of the old Context API. By making it a `Component`
instead of a `PureComponent`, and making sure there aren't any
disruptive ancestors between it and IntlProvider (in particular,
there are *no* ancestors in between) it'll be updated when `intl`
changes.

It already speaks the new Context API as well as it needs to (i.e.,
nothing special has to happen to get its consumers to update), and
we don't have any consumers that directly consume the old one
straight from `react-intl`.

One necessary tweak was to stop using `this._`, which is only set in
the constructor. (Reminiscent of b530f6c, when we were removing
the `key` hack for styles.)

Fixes: #1946

Author: chrisbobbe

docs/architecture/react.md

@@ -103,7 +103,8 @@ long as the code adheres to core Redux principles:
 ### Context
 
 We're on board with the current API where possible; there's a
-third-party library we use that isn't there yet.
+third-party library we use that isn't there yet, but we deal with that
+at the edge by "translating" the old API to the new one.
 
 #### Current Context API
 
@@ -164,29 +165,21 @@ updates to the context, and not just by the consumer's own
 We have to think about the legacy Context API in just one place. The
 `react-intl` library's `IntlProvider` uses it to provide the `intl`
 context. The only consumer of `intl` is
-`TranslationContextTranslator`, but we've made that a `PureComponent`,
-so it doesn't get updated when `intl` changes. Our workaround until
-now has been a "`key` hack":
-
-If `locale` changes, we make the entire React component tree at and
-below `IntlProvider` remount. (Not merely re-`render`: completely
-vanish and start over with `componentDidMount`; see the note at [this
-doc](https://5d4b5feba32acd0008d0df98--reactjs.netlify.app/docs/reconciliation.html)
-starting with "Keys should be stable, predictable, and unique".) We do
-this with the [`key`
-attribute](https://reactjs.org/docs/lists-and-keys.html), which isn't
-recommended for use except in lists.
-
-In the next commit, we stop using the `key` hack and instead make
-`TranslationContextTranslator` "speak" the old API better.
+`TranslationContextTranslator`, which "speaks" the old API by being
+the direct child of `IntlProvider` and by being a `Component`, not a
+`PureComponent` (whose under-the-hood `shouldComponentUpdate` would
+suppress updates on context changes)—all to make sure that it
+re-`render`s whenever `intl` changes. Then,
+`TranslationContextTranslator` is itself a provider, and it provides
+the same value, but it does so in the new Context API way. All its
+consumers are updated appropriately, which is what we want.
 
 ### The exception: `MessageList`
 
-We have one React component that we wrote (beyond `connect` calls) that
-deviates from the above design: `MessageList`.  This is the only
-component that extends plain `Component` rather than `PureComponent`,
-and it's the only component in which we implement
-`shouldComponentUpdate`.
+We have one React component that we wrote (beyond `connect` calls)
+that deviates from the above design: `MessageList`.  It extends plain
+`Component` rather than `PureComponent`, and it's the only component
+in which we implement `shouldComponentUpdate`.
 
 In fact, `MessageList` does adhere to the Pure Component Principle -- its
 `render` method is a pure function of `this.props` and `this.context`.  So

src/boot/TranslationProvider.js

@@ -1,5 +1,5 @@
 /* @flow strict-local */
-import React, { PureComponent } from 'react';
+import React, { PureComponent, Component } from 'react';
 import type { ComponentType, ElementConfig, Node as React$Node } from 'react';
 import { Text } from 'react-native';
 import { IntlProvider } from 'react-intl';
@@ -51,9 +51,37 @@ const makeGetText = (intl: IntlShape): GetText => {
  * new API.
  *
  * See https://reactjs.org/docs/context.html
- * vs. https://reactjs.org/docs/legacy-context.html .
+ * vs. https://reactjs.org/docs/legacy-context.html.
+ *
+ * Why do we need this? `IntlProvider` uses React's "legacy context
+ * API", deprecated since React 16.3, of which the docs say:
+ *
+ *   ## Updating Context
+ *
+ *   Don't do it.
+ *
+ *   React has an API to update context, but it is fundamentally
+ *   broken and you should not use it.
+ *
+ * It's broken because a consumer in the old way would never
+ * re-`render` on changes to the context if they, or any of their
+ * ancestors below the provider, implemented `shouldComponentUpdate`
+ * in a way that blocked updates from the context. This meant that
+ * neither the provider nor the consumer had the power to fix many
+ * non-re-`render`ing bugs. A very common context-update-blocking
+ * implementation of `shouldComponentUpdate` is the one
+ * `PureComponent` uses, so the effect is widespread.
+ *
+ * In the new way, `shouldComponentUpdate`s (as implemented by hand or
+ * by using `PureComponent`) in the hierarchy all the way down to the
+ * consumer (inclusive) are ignored when the context updates.
+ *
+ * Consumers should consume `TranslationContext` as it's provided
+ * here, so they don't have to worry about not updating when it
+ * changes.
  */
-class TranslationContextTranslator extends PureComponent<{|
+// This component MUST NOT be a `PureComponent`; see above.
+class TranslationContextTranslator extends Component<{|
   +children: React$Node,
 |}> {
   context: { intl: IntlShape };
@@ -62,11 +90,9 @@ class TranslationContextTranslator extends PureComponent<{|
     intl: () => null,
   };
 
-  _ = makeGetText(this.context.intl);
-
   render() {
     return (
-      <TranslationContext.Provider value={this._}>
+      <TranslationContext.Provider value={makeGetText(this.context.intl)}>
         {this.props.children}
       </TranslationContext.Provider>
     );
@@ -84,21 +110,7 @@ class TranslationProvider extends PureComponent<Props> {
     const { locale, children } = this.props;
 
     return (
-      /* `IntlProvider` uses React's "legacy context API", deprecated since
-       * React 16.3, of which the docs say:
-       *
-       *   ## Updating Context
-       *
-       *   Don't do it.
-       *
-       *   React has an API to update context, but it is fundamentally
-       *   broken and you should not use it.
-       *
-       * To work around that, we set `key={locale}` to force the whole tree
-       * to rerender if the locale changes.  Not cheap, but the locale
-       * changing is rare.
-       */
-      <IntlProvider key={locale} locale={locale} textComponent={Text} messages={messages[locale]}>
+      <IntlProvider locale={locale} textComponent={Text} messages={messages[locale]}>
         <TranslationContextTranslator>{children}</TranslationContextTranslator>
       </IntlProvider>
     );