simplify syncing props to state in ThemeProvider

.changeset/yellow-tools-call.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+avoid useeffect when syncing theme config

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={{

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')
+})

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
+}