Hello, apologies for the pull request out of the blue. react-xarrows is awesome and met an urgent need we had on our project. There are a few rough edges we'd like to work out if possible.
We've occasionally gotten into an infinite loop when putting updateXarrow in a hook dependency list. Reading the code, updateXarrow looks reference stable after effects have fired, which is great. However, before effects have fired, useXarrow() will return a new reference every render. This can lead to a render loop if updateXarrow is placed in a hook's dependency list. Stabilize it by making the fallback value a module-scope const. Keep in mind what the React documentation says about effects:
The function passed to useEffect will run after the render is committed to the screen.
React may also defer or batch renders, so reference stability is important; the identity of functions should remain stable as long as the variables they enclose don't change.
I believe the work done in <Xwrapper> doesn't need to be based on effects, but to keep things simple on my first contribution I'm just focusing on useXarrow.
Thank you for your time and attention, and for react-xarrows!
Eliav2
commented
1 year ago
Hello, apologies for the pull request out of the blue. react-xarrows is awesome and met an urgent need we had on our project. There are a few rough edges we'd like to work out if possible.
We've occasionally gotten into an infinite loop when putting
updateXarrowin a hook dependency list. Reading the code,updateXarrowlooks reference stable after effects have fired, which is great. However, before effects have fired,useXarrow()will return a new reference every render. This can lead to a render loop if updateXarrow is placed in a hook's dependency list. Stabilize it by making the fallback value a module-scope const. Keep in mind what the React documentation says about effects:React may also defer or batch renders, so reference stability is important; the identity of functions should remain stable as long as the variables they enclose don't change.
I believe the work done in
<Xwrapper>doesn't need to be based on effects, but to keep things simple on my first contribution I'm just focusing onuseXarrow.Thank you for your time and attention, and for react-xarrows!