fix: do not remount children of I18nProvider (#1501)

packages/react/src/I18nProvider.test.tsx

@@ -3,44 +3,58 @@ import { act, render } from "@testing-library/react"
 
 import { I18nProvider, useLingui } from "./I18nProvider"
 import { setupI18n } from "@lingui/core"
-// eslint-disable-next-line import/no-extraneous-dependencies
-import { mockConsole } from "@lingui/jest-mocks"
 
 describe("I18nProvider", () => {
-  it("should pass i18n context to wrapped component", () => {
-    const i18n = setupI18n({
-      locale: "cs",
-      messages: {
-        cs: {},
-      },
-    })
+  it(
+    "should pass i18n context to wrapped components, " +
+      "and re-render components that consume the context through useLingui()",
+    () => {
+      const i18n = setupI18n({
+        locale: "en",
+        messages: {
+          en: {},
+          cs: {},
+        },
+      })
+      let staticRenderCount = 0,
+        dynamicRenderCount = 0
+      const WithoutLinguiHook = (props) => {
+        staticRenderCount++
+        return <div {...props}>{props.i18n.locale}</div>
+      }
+
+      const WithLinguiHook = (props) => {
+        const { i18n } = useLingui()
+        dynamicRenderCount++
+        return <div {...props}>{i18n.locale}</div>
+      }
+
+      const { getByTestId } = render(
+        <I18nProvider i18n={i18n}>
+          <WithoutLinguiHook i18n={i18n} data-testid="static" />
+          <WithLinguiHook data-testid="dynamic" />
+        </I18nProvider>
+      )
 
-    const WithoutLingui = (props) => {
-      return <div {...props}>{props?.i18n?.locale}</div>
-    }
+      act(() => {
+        i18n.activate("cs")
+      })
 
-    const WithLingui = (props) => {
-      const { i18n } = useLingui()
-      return <WithoutLingui i18n={i18n} {...props} />
-    }
+      expect(getByTestId("static").textContent).toEqual("en")
+      expect(getByTestId("dynamic").textContent).toEqual("cs")
 
-    const { getByTestId } = render(
-      <I18nProvider i18n={i18n}>
-        <WithoutLingui data-testid="not-composed" />
-        <WithLingui data-testid="composed" />
-      </I18nProvider>
-    )
-
-    act(() => {
-      i18n.load("cs", {})
-      i18n.activate("cs")
-    })
+      act(() => {
+        i18n.activate("en")
+      })
 
-    expect(getByTestId("not-composed").textContent).toEqual("")
-    expect(getByTestId("composed").textContent).toEqual("cs")
-  })
+      expect(getByTestId("static").textContent).toEqual("en")
+      expect(getByTestId("dynamic").textContent).toEqual("en")
+      expect(staticRenderCount).toEqual(1)
+      expect(dynamicRenderCount).toEqual(3) // initial, cs, en
+    }
+  )
 
-  it("should subscribe for locale changes", () => {
+  it("should subscribe for locale changes upon mount", () => {
     const i18n = setupI18n({
       locale: "cs",
       messages: {
@@ -55,7 +69,7 @@ describe("I18nProvider", () => {
         <div />
       </I18nProvider>
     )
-    expect(i18n.on).toBeCalledWith("change", expect.anything())
+    expect(i18n.on).toBeCalledWith("change", expect.any(Function))
   })
 
   it("should unsubscribe for locale changes on unmount", () => {
@@ -78,50 +92,90 @@ describe("I18nProvider", () => {
     expect(unsubscribe).toBeCalled()
   })
 
-  it("should re-render on locale changes", async () => {
-    expect.assertions(4)
+  it("I18nProvider renders `null` until locale is activated. Children are rendered after activation.", () => {
+    expect.assertions(3)
 
-    const i18n = setupI18n({
-      messages: { en: {} },
-    })
+    const i18n = setupI18n()
 
-    const CurrentLocale = () => {
-      return <span>{i18n.locale}</span>
+    const CurrentLocaleStatic = () => {
+      return <span data-testid="static">1_{i18n.locale}</span>
+    }
+    const CurrentLocaleContextConsumer = () => {
+      const { i18n } = useLingui()
+      return <span data-testid="dynamic">2_{i18n.locale}</span>
     }
 
-    let container: HTMLElement
-
-    mockConsole((console) => {
-      const res = render(
-        <I18nProvider i18n={i18n}>
-          <CurrentLocale />
-        </I18nProvider>
-      )
-
-      container = res.container
-      expect(console.log.mock.calls[0][0]).toMatchInlineSnapshot(
-        `"I18nProvider did not render. A call to i18n.activate still needs to happen or forceRenderOnLocaleChange must be set to false."`
-      )
-    })
+    const { container } = render(
+      <I18nProvider i18n={i18n}>
+        <CurrentLocaleStatic />
+        <CurrentLocaleContextConsumer />
+      </I18nProvider>
+    )
 
-    // First render — no output, because locale isn't activated
+    // First render — locale isn't activated
     expect(container.textContent).toEqual("")
 
     act(() => {
-      i18n.load("en", {})
+      i18n.load("cs", {})
     })
-    // Again, no output. Catalog is loaded, but locale
-    // still isn't activated.
+    // Catalog is loaded, but locale still isn't activated.
     expect(container.textContent).toEqual("")
 
     act(() => {
-      i18n.load("cs", {})
       i18n.activate("cs")
     })
-    // After loading and activating locale, it's finally rendered.
-    expect(container.textContent).toEqual("cs")
+
+    // After loading and activating locale, components are rendered for the first time
+    expect(container.textContent).toEqual("1_cs2_cs")
   })
 
+  it(
+    "given 'en' locale, if activate('cs') call happens before i18n.on-change subscription in useEffect(), " +
+      "I18nProvider detects that it's stale and re-renders with the 'cs' locale value",
+    () => {
+      const i18n = setupI18n({
+        locale: "en",
+        messages: { en: {} },
+      })
+      let renderCount = 0
+
+      const CurrentLocaleContextConsumer = () => {
+        const { i18n } = useLingui()
+        renderCount++
+        return <span data-testid="child">{i18n.locale}</span>
+      }
+
+      /**
+       * Note that we're doing exactly what the description says:
+       * but to simulate the equivalent situation, we pass our own mock subscriber
+       * to i18n.on("change", ...) and in it we call i18n.activate("cs") ourselves
+       * so that the condition in useEffect() is met and the component re-renders
+       * */
+      const mockSubscriber = jest.fn(() => {
+        i18n.load("cs", {})
+        i18n.activate("cs")
+        return () => {
+          // unsubscriber - noop to make TS happy
+        }
+      })
+      jest.spyOn(i18n, "on").mockImplementation(mockSubscriber)
+
+      const { getByTestId } = render(
+        <I18nProvider i18n={i18n}>
+          <CurrentLocaleContextConsumer />
+        </I18nProvider>
+      )
+
+      expect(mockSubscriber).toHaveBeenCalledWith(
+        "change",
+        expect.any(Function)
+      )
+
+      expect(getByTestId("child").textContent).toBe("cs")
+      expect(renderCount).toBe(2)
+    }
+  )
+
   it("should render children", () => {
     const i18n = setupI18n({
       locale: "en",

packages/react/src/I18nProvider.tsx

@@ -8,11 +8,10 @@ export type I18nContext = {
 }
 
 export type I18nProviderProps = I18nContext & {
-  forceRenderOnLocaleChange?: boolean
   children?: React.ReactNode
 }
 
-const LinguiContext = React.createContext<I18nContext>(null)
+export const LinguiContext = React.createContext<I18nContext>(null)
 
 export function useLingui(): I18nContext {
   const context = React.useContext<I18nContext>(LinguiContext)
@@ -29,66 +28,64 @@ export function useLingui(): I18nContext {
 export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
   i18n,
   defaultComponent,
-  forceRenderOnLocaleChange = true,
   children,
 }) => {
+  const latestKnownLocale = React.useRef<string | undefined>(i18n.locale)
   /**
    * We can't pass `i18n` object directly through context, because even when locale
    * or messages are changed, i18n object is still the same. Context provider compares
-   * reference identity and suggested workaround is create a wrapper object every time
+   * reference identity and suggested workaround is to create a wrapper object every time
    * we need to trigger re-render. See https://reactjs.org/docs/context.html#caveats.
    *
    * Due to this effect we also pass `defaultComponent` in the same context, instead
    * of creating a separate Provider/Consumer pair.
    *
    * We can't use useMemo hook either, because we want to recalculate value manually.
    */
-  const makeContext = () => ({
-    i18n,
-    defaultComponent,
-  })
-  const getRenderKey = () => {
-    return (
-      forceRenderOnLocaleChange ? i18n.locale || "default" : "default"
-    ) as string
-  }
+  const makeContext = React.useCallback(
+    () => ({
+      i18n,
+      defaultComponent,
+    }),
+    [i18n, defaultComponent]
+  )
 
-  const [context, setContext] = React.useState<I18nContext>(makeContext()),
-    [renderKey, setRenderKey] = React.useState<string>(getRenderKey())
+  const [context, setContext] = React.useState<I18nContext>(makeContext())
 
   /**
    * Subscribe for locale/message changes
    *
    * I18n object from `@lingui/core` is the single source of truth for all i18n related
    * data (active locale, catalogs). When new messages are loaded or locale is changed
    * we need to trigger re-rendering of LinguiContext.Consumers.
-   *
-   * We call `setContext(makeContext())` after adding the observer in case the `change`
-   * event would already have fired between the inital renderKey calculation and the
-   * `useEffect` hook being called. This can happen if locales are loaded/activated
-   * async.
    */
   React.useEffect(() => {
-    const unsubscribe = i18n.on("change", () => {
+    const updateContext = () => {
+      latestKnownLocale.current = i18n.locale
       setContext(makeContext())
-      setRenderKey(getRenderKey())
-    })
-    if (renderKey === "default") {
-      setRenderKey(getRenderKey())
     }
-    if (forceRenderOnLocaleChange && renderKey === "default") {
-      console.log(
-        "I18nProvider did not render. A call to i18n.activate still needs to happen or forceRenderOnLocaleChange must be set to false."
-      )
+    const unsubscribe = i18n.on("change", updateContext)
+
+    /**
+     * unlikely, but if the locale changes before the onChange listener
+     * was added, we need to trigger a rerender
+     * */
+    if (latestKnownLocale.current !== i18n.locale) {
+      updateContext()
     }
-    return () => unsubscribe()
-  }, [])
+    return unsubscribe
+  }, [makeContext])
 
-  if (forceRenderOnLocaleChange && renderKey === "default") return null
+  if (!latestKnownLocale.current) {
+    process.env.NODE_ENV === "development" &&
+      console.log(
+        "I18nProvider rendered `null`. A call to `i18n.activate` needs to happen in order for translations to be activated and for the I18nProvider to render." +
+          "This is not an error but an informational message logged only in development."
+      )
+    return null
+  }
 
   return (
-    <LinguiContext.Provider value={context} key={renderKey}>
-      {children}
-    </LinguiContext.Provider>
+    <LinguiContext.Provider value={context}>{children}</LinguiContext.Provider>
   )
 }

packages/react/src/index.ts

@@ -1,4 +1,4 @@
-export { I18nProvider, useLingui } from "./I18nProvider"
+export { I18nProvider, useLingui, LinguiContext } from "./I18nProvider"
 
 export type { I18nProviderProps, I18nContext } from "./I18nProvider"