-
-
Save mjackson/fafef431758f45ba3b830ad9cbeb2328 to your computer and use it in GitHub Desktop.
/** | |
* Registers the given callback to be called in the node.js callback | |
* style, with error as the first argument, when the promise resolves. | |
* | |
* Also, returns a function that may be used to prevent the callback | |
* from ever being called. Calling the returned function is synonymous | |
* with saying "I no longer care about the resolution of this promise, | |
* even if it fails." | |
* | |
* Since there is no provision in the promise spec for cancel/abort | |
* behavior, this may be used as a workaround. | |
* | |
* In React components, you can use this function as a workaround for | |
* the deprecation of the isMounted() feature. | |
* | |
* const AsyncInput = React.createClass({ | |
* handleResponse(error, value) { | |
* this.setState(...) | |
* }, | |
* handleChange(event) { | |
* this.ignoreResponse = createBinding( | |
* makeRequest(event.target.value), | |
* this.handleResponse | |
* ) | |
* }, | |
* componentWillUnmount() { | |
* // Ignore any requests that are currently in progress. | |
* if (this.ignoreResponse) | |
* this.ignoreResponse() | |
* }, | |
* render() { | |
* return <input onChange={this.handleChange}/> | |
* } | |
* }) | |
*/ | |
export const createBinding = (promise, callback) => { | |
let isIgnored = false | |
promise.then( | |
value => !isIgnored && callback(null, value), | |
error => !isIgnored && callback(error) | |
) | |
return () => | |
isIgnored = true | |
} |
I disagree, @rnicholus. A lot of people share this idea (I've heard it a few times before) so please allow me to expand a little on why I disagree.
tl;dr: Our job here is to handle errors thrown by the promise, not the callback.
The whole promise paradigm is designed to model JavaScript's native try
/catch
behavior with an asynchronous call "stack". For purposes of illustration, let's assume that promise
is a synchronous operation, modeled with a function that either return
s or throw
s. If this were the case, my version of createBinding
would look something like this:
let value
let isError = false
try {
value = promise()
} catch (error) {
isError = true
value = error
}
if (isError) {
callback(value)
} else {
callback(null, value)
}
This is fairly straightforward: try
to call callback
with the return value of the operation, but catch
the error
and use it as the first argument if the operation fails. One key characteristic of this code is that it makes no attempt to catch
any errors that callback
itself might throw
. That's not its job. It's only supposed to resolve the promise
d value.
In the version you propose, with the second .catch
chained onto the end of the first .then
, the sync equivalent would be:
try {
callback(null, promise())
} catch (error) {
callback(error)
}
The .catch
in your example essentially says "catch everything, both errors in promise() and in callback()". We now have the following problems:
- Inside the
catch
block, how do you know where theerror
came from? Was it thrown bypromise()
? Or somewhere insidecallback()
? - If
callback
is the culprit:- How can we know it won't just
throw
again when we call it in thecatch
block? - Is it safe to call
callback
twice? We're calling it for the 2nd time inside thecatch
.
- How can we know it won't just
These are semantics that we need to communicate to consumers if we're going to catch
errors that callback
throws.
Probably a nitpick, but it seems the "rejected" callback function passed to
then
is sub-optimal to registering acatch
handler instead. The benefit ofcatch
over passing a second callback function to yourthen
call is thatcatch
will also be called if the "resolve" callback throws. So that results in the following adjustment tocreateBinding
:A nitpick since it arguably isn't important for a canned example like this, but it seems
catch
is a best practice, so perhaps it should be promoted in examples.