Memoize the whole Provider component instead of each prop

It simplifies the code a lot and might also make it faster, as memoization during render comes at the cost of additional array and function scope allocation.

This also adds a test to check that the Provider is not re-rendered when the bundles prop does not change.

Author: Gregoor

fluent-react/src/provider.js

@@ -1,5 +1,5 @@
 import { CachedSyncIterable } from "cached-iterable";
-import { createElement, useMemo } from "react";
+import { createElement, memo } from "react";
 import PropTypes from "prop-types";
 import { mapBundleSync } from "@fluent/sequence";
 import FluentContext from "./context";
@@ -23,7 +23,7 @@ import createParseMarkup from "./markup";
  * `ReactLocalization` to format translations.  If a translation is missing in
  * one instance, `ReactLocalization` will fall back to the next one.
  */
-export default function LocalizationProvider(props) {
+function LocalizationProvider(props) {
   if (props.bundles === undefined) {
     throw new Error("LocalizationProvider must receive the bundles prop.");
   }
@@ -32,52 +32,42 @@ export default function LocalizationProvider(props) {
     throw new Error("The bundles prop must be an iterable.");
   }
 
-  const bundles = useMemo(() => CachedSyncIterable.from(props.bundles), [
-    props.bundles
-  ]);
-  const parseMarkup = useMemo(() => props.parseMarkup || createParseMarkup(), [
-    props.parseMarkup
-  ]);
-  const contextValue = useMemo(
-    () => {
-      const l10n = {
-        getBundle: id => mapBundleSync(bundles, id),
-        getString(id, args, fallback) {
-          const bundle = l10n.getBundle(id);
+  const bundles = CachedSyncIterable.from(props.bundles);
+  const parseMarkup = props.parseMarkup || createParseMarkup();
+  const l10n = {
+    getBundle: id => mapBundleSync(bundles, id),
+    getString(id, args, fallback) {
+      const bundle = l10n.getBundle(id);
 
-          if (bundle) {
-            const msg = bundle.getMessage(id);
-            if (msg && msg.value) {
-              let errors = [];
-              let value = bundle.formatPattern(msg.value, args, errors);
-              for (let error of errors) {
-                l10n.reportError(error);
-              }
-              return value;
-            }
+      if (bundle) {
+        const msg = bundle.getMessage(id);
+        if (msg && msg.value) {
+          let errors = [];
+          let value = bundle.formatPattern(msg.value, args, errors);
+          for (let error of errors) {
+            l10n.reportError(error);
           }
-
-          return fallback || id;
-        },
-        // XXX Control this via a prop passed to the LocalizationProvider.
-        // See https://github.com/projectfluent/fluent.js/issues/411.
-        reportError(error) {
-          /* global console */
-          // eslint-disable-next-line no-console
-          console.warn(`[@fluent/react] ${error.name}: ${error.message}`);
+          return value;
         }
-      };
-      return {
-        l10n,
-        parseMarkup
-      };
+      }
+
+      return fallback || id;
     },
-    [bundles, parseMarkup]
-  );
+    // XXX Control this via a prop passed to the LocalizationProvider.
+    // See https://github.com/projectfluent/fluent.js/issues/411.
+    reportError(error) {
+      /* global console */
+      // eslint-disable-next-line no-console
+      console.warn(`[@fluent/react] ${error.name}: ${error.message}`);
+    }
+  };
 
   return createElement(
     FluentContext.Provider,
-    { value: contextValue },
+    { value: {
+      l10n,
+      parseMarkup
+    } },
     props.children
   );
 }
@@ -105,3 +95,5 @@ function isIterable(props, propName, componentName) {
     `The ${propName} prop supplied to ${componentName} must be an iterable.`
   );
 }
+
+export default memo(LocalizationProvider);

fluent-react/test/provider_valid.test.js

@@ -1,5 +1,5 @@
 import React from "react";
-import TestRenderer from "react-test-renderer";
+import TestRenderer, {act} from "react-test-renderer";
 import { LocalizationProvider } from "../src/index";
 
 describe("LocalizationProvider - validation", () => {
@@ -43,4 +43,14 @@ describe("LocalizationProvider - validation", () => {
       TestRenderer.create(<LocalizationProvider bundles={0} />);
     }).toThrow(/must be an iterable/);
   });
+
+  test("is memoized (no re-render) when props are the same", () => {
+    const bundles = [];
+    const spy = jest.spyOn(bundles, Symbol.iterator);
+    let renderer = TestRenderer.create(<LocalizationProvider bundles={bundles} />);
+    act(() => {
+      renderer = renderer.update(<LocalizationProvider bundles={bundles} />);
+    });
+    expect(spy).toHaveBeenCalledTimes(1);
+  });
 });