fix: eventsRef should trigger the lastest version of config callbacks#521
fix: eventsRef should trigger the lastest version of config callbacks#521shuding merged 4 commits intovercel:masterfrom
eventsRef should trigger the lastest version of config callbacks#521Conversation
* Currently,the config callbacks captured by the eventsRef is the callbacks created in the initial render of the component. * If the config callbacks is pure, everything is fine * but if the config callbacks rely on some variables of the components , it would cause some bugs related to the stale closure.
|
cc @sergiodxa |
|
Also, i've noticed some code paths that using the |
| ) | ||
|
|
||
| const lastestConfig = useRef(config) | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
I'm not sure if we need to have this useEffect here, could it just be updated in each rendering directly?
const configRef = useRef(config)
configRef.current = configThere was a problem hiding this comment.
@huozhi I feel that we still need to wrap it with useEffect (useIsomorphicLayoutEffect here). Updating refs directly in render can be dangerous in Concurrent Mode (not 100% about this specific usage here).
I'll approve this for now, but I suggest adding a comment to clarify 👍
There was a problem hiding this comment.
sure, good to know there's side effect on concurrent mode! 🙏 sorry for my unreasonable comment above
There was a problem hiding this comment.
@shuding thanks to point this out,do i need to change this back?
There was a problem hiding this comment.
yep, I think you can just simply revert the last commit. the rest looks good 👍
shuding
left a comment
There was a problem hiding this comment.
@lintuming Thank you so much for this great idea and also creating so many tests! Great work! 🙏
|
@shuding @huozhi @lintuming could we merge this? |
fix
Resolve: #508 #515
Now the
eventsRefread the config via theref,which always capture the lastest version of the config callbacks.This fix the problem of the stale closure related to the config callbacks.