Created
June 14, 2017 05:17
-
-
Save davidmason/2ad6fc4b35209a4fb6778cf3deeaead7 to your computer and use it in GitHub Desktop.
Debounce fails if you use it inside your callback. Here is how to fix it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
/* | |
* This is roughly the code I found. A callback is passed in props, | |
* and the author attempts to make a debounced version of the callback | |
* so that it will only trigger at most once every 200ms. | |
* | |
* Debounce is used when we are getting lots of events and want to | |
* avoid a costly operation (e.g. network request) on every one. It | |
* wraps a callback and returns a debounced callback that will only | |
* trigger the callback on the newest call when there is a break in | |
* activity. A common use-case is to request a search as a user types, | |
* but only send off the request when they pause in typing, rather | |
* than on every character. | |
* | |
* The problem here is that debounce is not used properly here. | |
* debounce returns a callback (wrapped around whatever callback you | |
* give it), and this code is never actually calling the callback. | |
*/ | |
class MyComponent extends Component { | |
static propTypes = { | |
handleSearch: PropTypes.func.isRequired, | |
} | |
constructor (props) { | |
super(props) | |
} | |
debounceHandleSearch = (searchText) => { | |
// debounce returns a callback, and this never calls it | |
debounce(() => { | |
this.props.handleSearch(searchText) | |
}, 200) | |
// we could add () to the end to call the returned callback | |
// but that creates a new debounced instance every time that | |
// debounceHandleSearch is called. Debounce works when you | |
// call it once to get a wrapped callback, then use the | |
// wrapped callback multiple times. | |
// The code would run, at least, but each call would run with | |
// a 200ms delay and we have defeated the purpose of using | |
// debounce. | |
} | |
onUpdateSearch = (event) => { | |
const searchText = event.target.value || '' | |
this.debounceHandleSearch(searchText) | |
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
/* | |
* The fix is to just debounce the function once, in the constructor. | |
* Note that the callback is passed to debounce directly without any | |
* wrapper function - debounce will pass the arguments through to | |
* the given callback so we can just call the debounced version the | |
* same way we would call the one passed in through props. | |
* | |
* Since the working of debounce has been misunderstood in this project | |
* before, it is worth including a comment to remind the team why it is | |
* done this way and warn against changing it back. | |
*/ | |
class MyComponent extends Component { | |
static propTypes = { | |
handleSearch: PropTypes.func.isRequired, | |
} | |
constructor (props) { | |
super(props) | |
// Do not extract this to a function like `() => { debounce(...) }` since | |
// that would make a new debounced function instance on every call. | |
this.debounceHandleSearch = debounce(props.handleSearch, 200) | |
} | |
onUpdateSearch = (event) => { | |
const searchText = event.target.value || '' | |
this.debounceHandleSearch(searchText) | |
} |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment