Skip to content

Commit

Permalink
Revert "refactor: use useSyncExternalStore to subscribe for context u…
Browse files Browse the repository at this point in the history
…pdates (#1746)" (#1755)

This reverts commit 47d4fdb.
  • Loading branch information
vonovak authored Aug 31, 2023
1 parent e7103c9 commit 4164893
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 94 deletions.
4 changes: 1 addition & 3 deletions packages/react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,12 @@
},
"dependencies": {
"@babel/runtime": "^7.20.13",
"@lingui/core": "4.4.1",
"use-sync-external-store": "^1.2.0"
"@lingui/core": "4.4.1"
},
"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",
Expand Down
79 changes: 47 additions & 32 deletions packages/react/src/I18nProvider.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,53 @@ 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",
Expand Down Expand Up @@ -179,36 +226,4 @@ 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")
})
})
65 changes: 24 additions & 41 deletions packages/react/src/I18nProvider.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
import React, {
ComponentType,
FunctionComponent,
useCallback,
useRef,
} from "react"
import { useSyncExternalStore } from "use-sync-external-store/shim"

import React, { ComponentType, FunctionComponent } from "react"
import type { I18n } from "@lingui/core"
import type { TransRenderProps } from "./TransNoContext"

Expand Down Expand Up @@ -38,6 +31,7 @@ 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
Expand All @@ -49,51 +43,42 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
*
* We can't use useMemo hook either, because we want to recalculate value manually.
*/
const makeContext = useCallback(
const makeContext = React.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 getSnapshot = useCallback(() => {
return context.current
}, [])
const [context, setContext] = React.useState<I18nContext>(makeContext())

/**
* Subscribe for locale/message changes
*
* the I18n object passed via props is the single source of truth for all i18n related
* 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 need to trigger re-rendering of LinguiContext.Consumers.
*/
const contextObject = useSyncExternalStore(
subscribe,
getSnapshot,
getSnapshot
)
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])

if (!contextObject.i18n.locale) {
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." +
Expand All @@ -103,8 +88,6 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
}

return (
<LinguiContext.Provider value={contextObject}>
{children}
</LinguiContext.Provider>
<LinguiContext.Provider value={context}>{children}</LinguiContext.Provider>
)
}
18 changes: 0 additions & 18 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3123,13 +3123,11 @@ __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
Expand Down Expand Up @@ -4397,13 +4395,6 @@ __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"
Expand Down Expand Up @@ -14833,15 +14824,6 @@ __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"
Expand Down

1 comment on commit 4164893

@vercel
Copy link

@vercel vercel bot commented on 4164893 Aug 31, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.