From edd567fa829438043535fe892cb29dc6a3d7e893 Mon Sep 17 00:00:00 2001
From: Matthew Costabile <mattcosta7@github.com>
Date: Fri, 16 Aug 2024 15:37:26 +0000
Subject: [PATCH 1/2] simplify syncing updates for theme provider
---
package-lock.json | 2 +-
packages/react/src/ThemeProvider.tsx | 22 ++-----
.../hooks/__tests__/useSyncedState.test.tsx | 63 +++++++++++++++++++
packages/react/src/hooks/useSyncedState.tsx | 28 +++++++++
4 files changed, 97 insertions(+), 18 deletions(-)
create mode 100644 packages/react/src/hooks/__tests__/useSyncedState.test.tsx
create mode 100644 packages/react/src/hooks/useSyncedState.tsx
diff --git a/package-lock.json b/package-lock.json
index 0904d9bc007..974ff76d890 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -63203,4 +63203,4 @@
}
}
}
-}
\ No newline at end of file
+}
diff --git a/packages/react/src/ThemeProvider.tsx b/packages/react/src/ThemeProvider.tsx
index e8647ef6a20..338294e54a2 100644
--- a/packages/react/src/ThemeProvider.tsx
+++ b/packages/react/src/ThemeProvider.tsx
@@ -4,6 +4,7 @@ import {ThemeProvider as SCThemeProvider} from 'styled-components'
import defaultTheme from './theme'
import deepmerge from 'deepmerge'
import {useId} from './hooks'
+import {useSyncedState} from './hooks/useSyncedState'
export const defaultColorMode = 'day'
const defaultDayScheme = 'light'
@@ -66,9 +67,9 @@ export const ThemeProvider: React.FC<React.PropsWithChildren<ThemeProviderProps>
const {resolvedServerColorMode} = getServerHandoff(uniqueDataId)
const resolvedColorModePassthrough = React.useRef(resolvedServerColorMode)
- const [colorMode, setColorMode] = React.useState(props.colorMode ?? fallbackColorMode ?? defaultColorMode)
- const [dayScheme, setDayScheme] = React.useState(props.dayScheme ?? fallbackDayScheme ?? defaultDayScheme)
- const [nightScheme, setNightScheme] = React.useState(props.nightScheme ?? fallbackNightScheme ?? defaultNightScheme)
+ const [colorMode, setColorMode] = useSyncedState(props.colorMode ?? fallbackColorMode ?? defaultColorMode)
+ const [dayScheme, setDayScheme] = useSyncedState(props.dayScheme ?? fallbackDayScheme ?? defaultDayScheme)
+ const [nightScheme, setNightScheme] = useSyncedState(props.nightScheme ?? fallbackNightScheme ?? defaultNightScheme)
const systemColorMode = useSystemColorMode()
const resolvedColorMode = resolvedColorModePassthrough.current || resolveColorMode(colorMode, systemColorMode)
const colorScheme = chooseColorScheme(resolvedColorMode, dayScheme, nightScheme)
@@ -101,22 +102,9 @@ export const ThemeProvider: React.FC<React.PropsWithChildren<ThemeProviderProps>
resolvedColorModePassthrough.current = null
}
},
- [colorMode, systemColorMode],
+ [colorMode, systemColorMode, setColorMode],
)
- // Update state if props change
- React.useEffect(() => {
- setColorMode(props.colorMode ?? fallbackColorMode ?? defaultColorMode)
- }, [props.colorMode, fallbackColorMode])
-
- React.useEffect(() => {
- setDayScheme(props.dayScheme ?? fallbackDayScheme ?? defaultDayScheme)
- }, [props.dayScheme, fallbackDayScheme])
-
- React.useEffect(() => {
- setNightScheme(props.nightScheme ?? fallbackNightScheme ?? defaultNightScheme)
- }, [props.nightScheme, fallbackNightScheme])
-
return (
<ThemeContext.Provider
value={{
diff --git a/packages/react/src/hooks/__tests__/useSyncedState.test.tsx b/packages/react/src/hooks/__tests__/useSyncedState.test.tsx
new file mode 100644
index 00000000000..78549ec82a1
--- /dev/null
+++ b/packages/react/src/hooks/__tests__/useSyncedState.test.tsx
@@ -0,0 +1,63 @@
+import {act, renderHook} from '@testing-library/react'
+import {useSyncedState} from '../useSyncedState'
+
+const renderUseSyncedState = (
+ initialValue: string | (() => string),
+ {isPropUpdateDisabled}: {isPropUpdateDisabled: boolean} = {isPropUpdateDisabled: false},
+) => {
+ return renderHook(props => useSyncedState(props.initialValue, {isPropUpdateDisabled: props.isPropUpdateDisabled}), {
+ initialProps: {
+ initialValue,
+ isPropUpdateDisabled,
+ },
+ })
+}
+test('it renders a default', () => {
+ const {result} = renderUseSyncedState('default')
+ expect(result.current[0]).toEqual('default')
+})
+
+test('it updates state from the internal state setter', () => {
+ const {result} = renderUseSyncedState('default')
+ expect(result.current[0]).toEqual('default')
+ act(() => {
+ result.current[1]('new value')
+ })
+ expect(result.current[0]).toEqual('new value')
+})
+
+test('it updates state from the internal state setter with an updater fn', () => {
+ const {result} = renderUseSyncedState('default')
+ expect(result.current[0]).toEqual('default')
+ act(() => {
+ result.current[1](prev => `${prev} new value`)
+ })
+ expect(result.current[0]).toEqual('default new value')
+})
+
+test('it updates state from the external state setter', () => {
+ const {result, rerender} = renderUseSyncedState('default')
+ expect(result.current[0]).toEqual('default')
+
+ rerender({initialValue: 'new value', isPropUpdateDisabled: false})
+
+ expect(result.current[0]).toEqual('new value')
+})
+
+test('it properly handles init functions', () => {
+ const {result, rerender} = renderUseSyncedState(() => 'default')
+ expect(result.current[0]).toEqual('default')
+
+ rerender({initialValue: () => 'new value', isPropUpdateDisabled: false})
+
+ expect(result.current[0]).toEqual('new value')
+})
+
+test('it does not update from props if disabled', () => {
+ const {result, rerender} = renderUseSyncedState('default', {isPropUpdateDisabled: true})
+ expect(result.current[0]).toEqual('default')
+
+ rerender({initialValue: 'new value', isPropUpdateDisabled: true})
+
+ expect(result.current[0]).toEqual('default')
+})
diff --git a/packages/react/src/hooks/useSyncedState.tsx b/packages/react/src/hooks/useSyncedState.tsx
new file mode 100644
index 00000000000..748f442bf22
--- /dev/null
+++ b/packages/react/src/hooks/useSyncedState.tsx
@@ -0,0 +1,28 @@
+import {useState} from 'react'
+
+/**
+ * When the value that initialized the state changes
+ * this hook will update the state to the new value, immediately.
+ *
+ * This uses an Object.is comparison to determine if the value has changed by default
+ *
+ * If you use a non-primitive value as the initial value, you should provide a custom isEqual function
+ *
+ * This is adapted almost directly from https://beta.reactjs.org/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes
+ */
+
+export const useSyncedState = <T,>(
+ initialValue: T | (() => T),
+ {isPropUpdateDisabled = false, isEqual = Object.is} = {},
+) => {
+ const [state, setState] = useState(initialValue)
+ const [previous, setPrevious] = useState(initialValue)
+
+ const nextInitialValue = initialValue instanceof Function ? initialValue() : initialValue
+ if (!isPropUpdateDisabled && !isEqual(previous, nextInitialValue)) {
+ setPrevious(nextInitialValue)
+ setState(nextInitialValue)
+ }
+
+ return [state, setState] as const
+}
From 6c5af04c5e0d5d12772f45c5a94dda63facd6b7b Mon Sep 17 00:00:00 2001
From: Matthew Costabile <mattcosta7@github.com>
Date: Fri, 16 Aug 2024 15:40:29 +0000
Subject: [PATCH 2/2] simplify syncing updates for theme provider
---
.changeset/yellow-tools-call.md | 5 +++++
1 file changed, 5 insertions(+)
create mode 100644 .changeset/yellow-tools-call.md
diff --git a/.changeset/yellow-tools-call.md b/.changeset/yellow-tools-call.md
new file mode 100644
index 00000000000..0b868ed40d6
--- /dev/null
+++ b/.changeset/yellow-tools-call.md
@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+avoid useeffect when syncing theme config