Fix debounced calls after unmount in responsive components #558
Conversation
There was a problem hiding this comment.
thanks for the fix @claisne! Looks good overall but I had a couple of suggestions.
I think the build is failing for an unrelated reason, will try to get that fixed asap.
| @@ -53,6 +53,7 @@ export default function withParentSize<Props extends WithParentSizeProps = {}>( | |||
| componentWillUnmount() { | |||
| if (this.animationFrameID) window.cancelAnimationFrame(this.animationFrameID); | |||
| if (this.resizeObserver) this.resizeObserver.disconnect(); | |||
| if (this.resize) this.resize.cancel(); | |||
There was a problem hiding this comment.
ditto re removing the if (...) statement since it's defined in the constructor.
also I think these need to be updated to this.debouncedResize/this.debouncedResize.cancel() because the unwrapped method wouldn't have the .cancel() method (nice that we name this differently in every component 😱 )
| @@ -37,6 +37,7 @@ export default function withScreenSize<Props extends WithScreenSizeProps = {}>( | |||
|
|
|||
| componentWillUnmount() { | |||
| window.removeEventListener('resize', this.handleResize, false); | |||
| this.resize.cancel(); | |||
There was a problem hiding this comment.
ditto re updating this to this.handleResize.cancel() since the unwrapped method wouldn't have the .cancel() method
| @@ -62,6 +62,7 @@ export default class ParentSize extends React.Component< | |||
| componentWillUnmount() { | |||
| if (this.animationFrameID) window.cancelAnimationFrame(this.animationFrameID); | |||
| if (this.resizeObserver) this.resizeObserver.disconnect(); | |||
| if (this.resize) this.resize.cancel(); | |||
There was a problem hiding this comment.
Is the if (...) statement needed here? since the debounced method is defined in the constructor I think this check is unneeded (versus animationFrameID/resizeObserver which can be null or undefined)
|
@claisne thanks for making the updates, I think if you rebase or merge the latest master the build should be fixed. |
🐛 Bug Fix
Debounce calls can still fire after unmount, so we need to cancel them.