CharlesStover / reactn

React, but with built-in global state management.
https://www.npmjs.com/package/reactn
MIT License
1.9k stars 85 forks source link

Dispatch re-renders components even when state is unchanged. #215

Open js-jslog opened 2 years ago

js-jslog commented 2 years ago

I am using reactn on 2.2.7 and react on 16.13.1

The react docs on useReducer imply that the state property being dispatched to will not actually be updated if the next value is identical to the previous one: https://reactjs.org/docs/hooks-reference.html#bailing-out-of-a-dispatch (it talks about components not rerendering but presumably the ultimate cause of that is that the state has not been updated)

It looks to me as though this same convention is not being observed in the reactn useDispatch with a global inline property reducer (https://github.com/CharlesStover/reactn#usedispatchfunction-keyof-state) which I would consider an equivalent use case.

// use-dispatch.js - when this dispatch is run...
...
const dispatch = useDispatch((prevStateItem) => prevStateItem, 'stateItem')
dispatch()
...
// component.jsx - ...this component is rerendered
...
const [stateItem] = useGlobal('stateItem')
...

I'm curious to know whether this is known and deliberate, unknown and of interest, or just plain wrongly observed of me.

I'm happy to provide more detailed explanation and examples if it would be helpful.

Thank you for the reactn project. It's been a joy to work with, not least because of the excellent documentation and articles. It's very intuitive and follows from reacts hooks nicely which is obviously a goal of the project. Congratulations.

quisido commented 2 years ago

Unknown and of interest. Ideally, the component would not re-render in this case.

I wouldn't consider this high enough priority to fix at the moment (a soon React release will add a store observer hook that would offer a better API for listening to state changes), but I'd be open to a PR assuming it is simple enough of a change to ensure no side effects.

Thanks for noticing and reporting this. I'm glad to have it documented.

js-jslog commented 2 years ago

Hi Charles,

Thanks for the roadmap headsup on this. That's interesting to know.

I won't have time to study the code and make a PR, but I can share my workaround for those who need one for performance reasons.

I had the dispatch in question update a different global property which isn't associated with any renders, which means it can afford to be updated on every dispatch with minimal cost. I then setup a useEffect which triggers each time that property is updated, but then only conditionally calls a setter from useGlobal if the object is identical to the previous.

You can set up some boilerplate to make this process reasonably generic, readable and maintainable. You can study the code changes in the PRs mentioned in the previous 2 issue comments here but there are a lot of changes there and a lot of context to dig through so if anyone actually needs this help let me know and I'll put a bit of thought in to breaking it down.