refactor: use useSyncExternalStore to subscribe for context updates (l…

…ingui#1746)
cultureamp · Aug 18, 2023 · 47d4fdb · 47d4fdb
1 parent 8232e13
commit 47d4fdb
Show file tree

Hide file tree

Showing 4 changed files with 94 additions and 72 deletions.
diff --git a/packages/react/package.json b/packages/react/package.json
@@ -63,12 +63,14 @@
   },
   "dependencies": {
     "@babel/runtime": "^7.20.13",
-    "@lingui/core": "4.4.0"
+    "@lingui/core": "4.4.0",
+    "use-sync-external-store": "^1.2.0"
   },
   "devDependencies": {
     "@lingui/jest-mocks": "*",
     "@testing-library/react": "^14.0.0",
     "@types/react": "^18.2.13",
+    "@types/use-sync-external-store": "^0.0.3",
     "eslint-plugin-react": "^7.32.2",
     "eslint-plugin-react-hooks": "^4.6.0",
     "react": "^18.2.0",

diff --git a/packages/react/src/I18nProvider.test.tsx b/packages/react/src/I18nProvider.test.tsx
@@ -132,53 +132,6 @@ describe("I18nProvider", () => {
     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",
@@ -226,4 +179,36 @@ describe("I18nProvider", () => {
 
     expect(getByText("Ahoj světe")).toBeTruthy()
   })
+
+  it("when re-rendered with new i18n instance, it will forward it to children", () => {
+    const i18nCs = setupI18n({
+      locale: "cs",
+      messages: { cs: {} },
+    })
+
+    const i18nEn = setupI18n({
+      locale: "en",
+      messages: { en: {} },
+    })
+
+    const CurrentLocaleContextConsumer = () => {
+      const { i18n } = useLingui()
+      return <span data-testid="dynamic">{i18n.locale}</span>
+    }
+
+    const { container, rerender } = render(
+      <I18nProvider i18n={i18nCs}>
+        <CurrentLocaleContextConsumer />
+      </I18nProvider>
+    )
+
+    expect(container.textContent).toEqual("cs")
+
+    rerender(
+      <I18nProvider i18n={i18nEn}>
+        <CurrentLocaleContextConsumer />
+      </I18nProvider>
+    )
+    expect(container.textContent).toEqual("en")
+  })
 })
diff --git a/packages/react/src/I18nProvider.tsx b/packages/react/src/I18nProvider.tsx
@@ -1,4 +1,11 @@
-import React, { ComponentType, FunctionComponent } from "react"
+import React, {
+  ComponentType,
+  FunctionComponent,
+  useCallback,
+  useRef,
+} from "react"
+import { useSyncExternalStore } from "use-sync-external-store/shim"
+
 import type { I18n } from "@lingui/core"
 import type { TransRenderProps } from "./TransNoContext"
 
@@ -31,7 +38,6 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
   defaultComponent,
   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
@@ -43,42 +49,51 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
    *
    * We can't use useMemo hook either, because we want to recalculate value manually.
    */
-  const makeContext = React.useCallback(
+  const makeContext = useCallback(
     () => ({
       i18n,
       defaultComponent,
       _: i18n.t.bind(i18n),
     }),
     [i18n, defaultComponent]
   )
+  const context = useRef<I18nContext>(makeContext())
+
+  const subscribe = useCallback(
+    (onStoreChange: () => void) => {
+      const renderWithFreshContext = () => {
+        context.current = makeContext()
+        onStoreChange()
+      }
+      const propsChanged =
+        context.current.i18n !== i18n ||
+        context.current.defaultComponent !== defaultComponent
+      if (propsChanged) {
+        renderWithFreshContext()
+      }
+      return i18n.on("change", renderWithFreshContext)
+    },
+    [makeContext, i18n, defaultComponent]
+  )
 
-  const [context, setContext] = React.useState<I18nContext>(makeContext())
+  const getSnapshot = useCallback(() => {
+    return context.current
+  }, [])
 
   /**
    * Subscribe for locale/message changes
    *
-   * I18n object from `@lingui/core` is the single source of truth for all i18n related
+   * the I18n object passed via props 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 need to trigger re-rendering of LinguiContext consumers.
    */
-  React.useEffect(() => {
-    const updateContext = () => {
-      latestKnownLocale.current = i18n.locale
-      setContext(makeContext())
-    }
-    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
-  }, [i18n, makeContext])
+  const contextObject = useSyncExternalStore(
+    subscribe,
+    getSnapshot,
+    getSnapshot
+  )
 
-  if (!latestKnownLocale.current) {
+  if (!contextObject.i18n.locale) {
     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." +
@@ -88,6 +103,8 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
   }
 
   return (
-    <LinguiContext.Provider value={context}>{children}</LinguiContext.Provider>
+    <LinguiContext.Provider value={contextObject}>
+      {children}
+    </LinguiContext.Provider>
   )
 }
diff --git a/yarn.lock b/yarn.lock
@@ -3123,11 +3123,13 @@ __metadata:
     "@lingui/jest-mocks": "*"
     "@testing-library/react": ^14.0.0
     "@types/react": ^18.2.13
+    "@types/use-sync-external-store": ^0.0.3
     eslint-plugin-react: ^7.32.2
     eslint-plugin-react-hooks: ^4.6.0
     react: ^18.2.0
     react-dom: ^18.2.0
     unbuild: ^1.1.2
+    use-sync-external-store: ^1.2.0
   peerDependencies:
     react: ^16.8.0 || ^17.0.0 || ^18.0.0
   languageName: unknown
@@ -4395,6 +4397,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@types/use-sync-external-store@npm:^0.0.3":
+  version: 0.0.3
+  resolution: "@types/use-sync-external-store@npm:0.0.3"
+  checksum: 161ddb8eec5dbe7279ac971531217e9af6b99f7783213566d2b502e2e2378ea19cf5e5ea4595039d730aa79d3d35c6567d48599f69773a02ffcff1776ec2a44e
+  languageName: node
+  linkType: hard
+
 "@types/yargs-parser@npm:*":
   version: 21.0.0
   resolution: "@types/yargs-parser@npm:21.0.0"
@@ -14824,6 +14833,15 @@ __metadata:
   languageName: node
   linkType: hard
 
+"use-sync-external-store@npm:^1.2.0":
+  version: 1.2.0
+  resolution: "use-sync-external-store@npm:1.2.0"
+  peerDependencies:
+    react: ^16.8.0 || ^17.0.0 || ^18.0.0
+  checksum: 5c639e0f8da3521d605f59ce5be9e094ca772bd44a4ce7322b055a6f58eeed8dda3c94cabd90c7a41fb6fa852210092008afe48f7038792fd47501f33299116a
+  languageName: node
+  linkType: hard
+
 "util-deprecate@npm:^1.0.1, util-deprecate@npm:~1.0.1":
   version: 1.0.2
   resolution: "util-deprecate@npm:1.0.2"