Skip to content

Commit

Permalink
🐛 calling setValue() in useLayoutEffect() doesn't work
Browse files Browse the repository at this point in the history
this also introduces a breaking change because now if you call setValue() during render and you have two instances of the hook with the same hook React will call console.error() with the following message: "Warning: Cannot update a component (`Component`) while rendering a different component (`Component`). To locate the bad setState() call inside `Component`, follow the stack trace as described in https://reactjs.org/link/setstate-in-render.
  • Loading branch information
astoilkov committed Mar 25, 2022
1 parent baa4ca1 commit 65614f3
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 20 deletions.
38 changes: 19 additions & 19 deletions src/useLocalStorageState.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import storage from './storage'
import { unstable_batchedUpdates } from 'react-dom'
import { useRef, useMemo, useEffect, useReducer, useCallback } from 'react'
import type { Dispatch, SetStateAction } from 'react'
import { useRef, useMemo, useEffect, useReducer, useCallback } from 'react'

// `activeHooks` holds all active hooks. we use the array to update all hooks with the same key —
// calling `setValue` of one hook triggers an update for all other hooks with the same key
const activeHooks: {
const activeHooks = new Set<{
key: string
forceUpdate: () => void
}[] = []
}>()

// - `useLocalStorageState()` return type
// - first two values are the same as `useState`
Expand Down Expand Up @@ -68,13 +68,9 @@ function useClientLocalStorageState<T>(
storage.set(key, newUnwrappedValue)

unstable_batchedUpdates(() => {
// 🐛 `setValue()` during render doesn't work
// https://github.com/astoilkov/use-local-storage-state/issues/43
forceUpdate()

for (const update of activeHooks) {
if (update.key === key) {
update.forceUpdate()
for (const hook of activeHooks) {
if (hook.key === key) {
hook.forceUpdate()
}
}
})
Expand All @@ -97,15 +93,19 @@ function useClientLocalStorageState<T>(
return (): void => window.removeEventListener('storage', onStorage)
}, [key])

// add this hook to the `activeHooks` array. see the `activeHooks` declaration above for a
// more detailed explanation
useEffect(() => {
const entry = { key, forceUpdate }
activeHooks.push(entry)
return (): void => {
activeHooks.splice(activeHooks.indexOf(entry), 1)
}
}, [key])
// - adds this hook to the `activeHooks` array. see the `activeHooks` declaration above for a
// more detailed explanation
// - not inside a React effect because:
// - it fixes "🐛 `setValue()` during render doesn't work":
// https://github.com/astoilkov/use-local-storage-state/issues/43
// - if you call `setValue()` during render and you have two localStorage instances with the
// same key — React throws an error. we want this behavior because otherwise it will just
// silently fail
const activeHookRef = useRef({ key, forceUpdate })
activeHooks.delete(activeHookRef.current)
activeHookRef.current = { key, forceUpdate }
activeHooks.add(activeHookRef.current)
useEffect(() => () => void activeHooks.delete(activeHookRef.current), [])

// - SSR support
// - not inside a `useLayoutEffect` because this way we skip the calls to `useEffect()` and
Expand Down
56 changes: 55 additions & 1 deletion test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ import util from 'node:util'
import storage from './src/storage'
import useLocalStorageState from '.'
import { render } from '@testing-library/react'
import React, { useEffect, useMemo } from 'react'
import { renderHook, act } from '@testing-library/react-hooks'
import React, { useEffect, useLayoutEffect, useMemo } from 'react'
import { renderHook as renderHookOnServer } from '@testing-library/react-hooks/server'

beforeEach(() => {
Expand Down Expand Up @@ -578,6 +578,60 @@ describe('createLocalStorageStateHook()', () => {
expect(queryByText(/^1$/u)).toBeTruthy()
})

it(`calling setValue() during render when there are two instances of useLocalStorageState() with the same key should throw an error`, () => {
function App() {
return (
<>
<Component update={false} />
<Component update={true} />
</>
)
}

function Component({ update }: { update: boolean }) {
const [value, setValue] = useLocalStorageState('number', {
defaultValue: 0,
})

if (update && value === 0) {
setValue(1)
}

return <div>{value}</div>
}

expect(() => render(<App />)).toThrow()
})

it(`calling setValue() from useLayoutEffect() should update all useLocalStorageState() instances`, () => {
function App() {
return (
<>
<Component update={false} />
<Component update={true} />
</>
)
}

function Component({ update }: { update: boolean }) {
const [value, setValue] = useLocalStorageState('number', {
defaultValue: 0,
})

useLayoutEffect(() => {
if (update) {
setValue(1)
}
}, [])

return <div>{value}</div>
}

const { queryAllByText } = render(<App />)

expect(queryAllByText(/^1$/u)).toHaveLength(2)
})

describe('SSR support', () => {
beforeEach(() => {
const windowSpy = jest.spyOn(global, 'window' as any, 'get')
Expand Down

0 comments on commit 65614f3

Please sign in to comment.