-
Notifications
You must be signed in to change notification settings - Fork 103
Reducing components rerenders #53
Comments
It will be unsafe to mutate refs in the render phase when concurrent mode arrives . You can read more on why here #9 for example. In short a render might be aborted and the commit won't happen so you might end up in a situation where you have a ref pointing to a future state that hasn't been committed yet. Do you have 6 rerenders because you dispatch 4 times in the body of the component? Have you considered using 1 action that does what all 4 do? |
@Turanchoks shouldn't
Yea, I can fix this many ways, I'm just saying that for 4 state mutations, having multiple store subscriptions ( |
So there are actual commits between the dispatches in your case, right? I originally used useLayoutEffect too as it solved a different issue which is no longer a problem now. There is a whole paragraph https://github.com/reduxjs/react-redux/blob/06a674e733454b08bb4090f8bcb038a3a003e6aa/src/hooks/useSelector.js#L7 that describes why they use useLayoutEffect but I'm not sure I fully understand the issue. There is no overhead in using useLayoutEffect instead of useEffect so I think we can change it. A test case would be great. Maybe @markerikson could help. Is there a test case in the react-redux repo or anywhere that covers a problem of using useEffect in https://github.com/reduxjs/react-redux/blob/06a674e733454b08bb4090f8bcb038a3a003e6aa/src/hooks/useSelector.js#L85? Thanks |
@Turanchoks |
The original implementation |
const Home = ({navigation}) => { const [loading, setLoading] = useState(true); useLayoutEffect(() => {
}, [data]); this functional component rander three times when i dispatch a action with redux, can anyone tell me whats wrong with this one ? |
@arwysyah A codesandbox link would make your problem easier to diagnose, as the code doesn't seem to make sense on its own. |
Looks like we can reduce component rerenders count by removing
useEffect
from herehttps://github.com/facebookincubator/redux-react-hook/blob/676a270be12f0bcbffe9d650ef769d6dd486ef70/src/create.ts#L91-L94
Since
useEffect
will execute somewhen in the future after component renders, even if our hook already returned new state,https://github.com/facebookincubator/redux-react-hook/blob/676a270be12f0bcbffe9d650ef769d6dd486ef70/src/create.ts#L110
still compares to old.
In practice, for my case, with using 6 hooks and 4 dispatches, it's reducing rerenders counts from 6 to 4.
here is used useLayoutEffect
https://github.com/reduxjs/react-redux/blob/06a674e733454b08bb4090f8bcb038a3a003e6aa/src/hooks/useSelector.js#L79-L85
Which seems to work correctly too.
The text was updated successfully, but these errors were encountered: