streamich / react-use

React Hooks — 👍
http://streamich.github.io/react-use
The Unlicense
41.8k stars 3.15k forks source link

`useMethods` methods object is not stable and changes when `initialState` is changed #1286

Open BenJenkinson opened 4 years ago

BenJenkinson commented 4 years ago

What is the current behavior?

The useMethods hook wraps the behaviour of useReducer to return a nicely typed methods object.

https://github.com/streamich/react-use/blob/0fabbcd641e5fc6d16091e7c4c803efe80d7ff56/docs/useMethods.md#L44

However, the methods object returned from useMethods is not stable and changes whenever the initialState argument changes.

This does not match how React treats an initialState argument for useState and useReducer

The initialState argument is the state used during the initial render. In subsequent renders, it is disregarded. (https://reactjs.org/docs/hooks-reference.html#lazy-initial-state)

This also does not match how React guarantees the setState method from useState and the dispatch method from useReducer to both be stable and not to change on re-renders or when initialState changes.

What is the expected behavior?

I would expect that the methods object was guaranteed to be stable and not to change on re-renders or when initialState changes. (Though I would expect it to change if the createMethods argument changed)

This would then match the behaviour of the setState method from useState and the dispatch method from useReducer.

Investigation

Looking at the source of useMethods.ts, I think the current behaviour is unintentional.

For example, the dispatch behaviour is already stable, since createMethods is re-run for each dispatch call:

https://github.com/streamich/react-use/blob/0fabbcd641e5fc6d16091e7c4c803efe80d7ff56/src/useMethods.ts#L19-L24

But the way that the wrappedMethods object is created introduces a dependency on initialState:

https://github.com/streamich/react-use/blob/0fabbcd641e5fc6d16091e7c4c803efe80d7ff56/src/useMethods.ts#L28-L35

I think the bug was introduced accidentally in https://github.com/streamich/react-use/commit/b394f3d72356d331dbce48acd3686bbb64d331b5, since it wasn't present in the original version of the hook.

Version Info

tiwariav commented 3 years ago

Faced a simillar issue, where the methods object was changing on each render because of the initialState.

function SomeProvider({ children, data }) {
  const [state, methods] = useMethods(createMethods, { ...INITIAL_STATE, ...data });

Was able to fix it by memoizing the initialState as

function SomeProvider({ children, data }) {
  const initialState = useMemo(
    () => ({ ...INITIAL_STATE, ...data }),
    [data]
  );
  const [state, methods] = useMethods(createMethods, initialState);
danguilherme commented 2 years ago

Just stumbled upon this. Seems like a basic design flaw, initialState value shouldn't be tracked.

dbousamra commented 2 years ago

Also affected.

vin-mfvhn commented 1 year ago

Looks like they fixed this in the master:

https://github.com/streamich/react-use/blob/master/src/useMethods.ts

danguilherme commented 1 year ago

Nope, initialState is still being tracked for the creation of the methods: https://github.com/streamich/react-use/blob/325f5bd69904346788ea981ec18bfc7397c611df/src/useMethods.ts?rgh-link-date=2022-12-20T13%3A34%3A27Z#L36

It was added in Feb 2020 for some reason: https://github.com/streamich/react-use/commit/b394f3d72356d331dbce48acd3686bbb64d331b5

danguilherme commented 1 year ago

I opened a PR fixing it: https://github.com/streamich/react-use/pull/2458