Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

safely access this.input.value #419

Merged
merged 1 commit into from Feb 24, 2021
Merged

Conversation

mattkrick
Copy link

not really sure how to reproduce, but apparently one of my users managed to time their search perfectly (probably due to the throttle?).

this should prevent that.

Screen Shot 2020-03-18 at 11 15 10 AM

@h6ak
Copy link

h6ak commented Apr 26, 2020

This PR will resolve #360 .

@CoryDanielson
Copy link

CoryDanielson commented Aug 18, 2020

This fix will work (thank you!)

An alternative to this is to update throttleIdleTask to return a cancel method on the returned function that you can call on componentWillUnmount. This is available when using throttle and debounce in lodash so it's sort of a common convention with debounced/throttled stuff.

https://lodash.com/docs/4.17.15#throttle

Creates a throttled function that only invokes func at most once per every wait milliseconds. The throttled function comes with a cancel method to cancel delayed func invocations and a flush method to immediately invoke them.

Usage:

constructor(props) {
  // ...
  this.handleChange = throttleIdleTask(this.handleChange.bind(this))
}

componentWillUnmount() {
  this.handleChange.cancel();
}

This would take more effort but is generally a cleaner fix because it allows you to write your callbacks without having to handle the scenario where a component may be unmounted (assuming you remember to cancel them on componentWillUnmount). Another benefit is if you ever remove a debounce/timeout, you're likely not going to update the callback to stop null checking. That code is assumed to be useful and easy to leave behind - the code that was originally guarding against a debounce/timeout becomes crufty code that actually has no use but is assumed to be useful 😬

/2 cents from someone who discovered this error in Sentry today

@felipevegaaraujo
Copy link

Can we get this fix merged/released?

@EtienneLem EtienneLem merged commit cc257c7 into missive:master Feb 24, 2021
@EtienneLem
Copy link
Member

Thanks 🤘🏼, sorry for the delay. v3.0.1 has just been released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants