react-mapbox-gl
react-mapbox-gl copied to clipboard
Fix memoized rebind
This is a strawman fix for #927. Currently react-mapbox-gl binds DOM events the first time a prop function is passed in, which means that for memoized functions (for instance, with useMemo / React functional components) the handler may be a stale closure and have out-of-date data.
The issue is that the updateEvents function in map-events.tsx only checks for the presence of the function but not referential equality, and doesn't dereference the latest value off of this.props.
As I see it, there are 3 approaches we could take to fix this in library code:
- Unbind stale functions and re-bind new functions when passed in (i.e. change the
toListenOffexpression to check against prevProps -- see the first commit here). -
- Dereference the latest value of the callback in this.props (the approach I took here in the second commit). The issue with this approach is that we need access to the
thisobject inupdateEvents, because thepropschanges between renders, so this is a bit uglier than I'd like.
- Dereference the latest value of the callback in this.props (the approach I took here in the second commit). The issue with this approach is that we need access to the
- Use a Ref, always set to the latest passed-in prop value. This seems like a duplication of 2 with extra steps.
The last approach would be to migrate map.tsx to be a functional component, but that might be a bigger ask.
Any suggestions here? Thanks!