Reducing components rerenders

[goloveychuk]

Looks like we can reduce component rerenders count by removing useEffect from here
https://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.

[Turanchoks]

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?

[goloveychuk]

@Turanchoks shouldn't useLayoutEffect work correctly? It should execute synchronously after mutating dom.

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?

Yea, I can fix this many ways, I'm just saying that for 4 state mutations, having multiple store subscriptions (useMappedState hooks), I have 6 component rerenders.
With both removing useEffect (which is bad idea, as you said) and using useLayoutEffect I have 4.

[Turanchoks]

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

[goloveychuk]

@Turanchoks
#54
this test should cover this case. The reason I removed act is because, it seems to commit useEffect hook synchronously, which is not how it works in real apps.
If you remove useLayoutEffect - test will fail with 5 rerenders.

[markerikson]

The original implementation connect in v7.0 used useEffect(), but we ran into issues with the timing of subscriptions, so we had to change that to useLayoutEffect()` as well:

reduxjs/react-redux#1226
reduxjs/react-redux#1235

[arwysyah]

const Home = ({navigation}) => {
const globalState = useSelector(state=>state)

const [loading, setLoading] = useState(true);
const [data, setData] = useState([]);
const dispatch = useDispatch()
useLayoutEffect(()=>{
dispatch(watchData())
},[dispatch])

useLayoutEffect(() => {
setTimeout(() => {
setData(data);

  setLoading(false);
}, 1000);

}, [data]);
console.log(globalState.request)
return()}

this functional component rander three times when i dispatch a action with redux, can anyone tell me whats wrong with this one ?

[ianobermiller]

@arwysyah A codesandbox link would make your problem easier to diagnose, as the code doesn't seem to make sense on its own.