Skip to content

RFC: More flow-friendly thenable checking. #699

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

Merged
merged 1 commit into from
Jan 27, 2017
Merged

RFC: More flow-friendly thenable checking. #699

merged 1 commit into from
Jan 27, 2017

Conversation

leebyron
Copy link
Contributor

This replaces the isThenable boolean check with a "getIfPromise" function. It's essentially the same, but rather than returning boolean, it's the identify function when it would have returned true, and returns void otherwise. That lets us give it a better type signature that Flow can do more with.

Open to feedback on this one, and open to bikeshedding on the name of this new function.

@wincent
Copy link
Contributor

wincent commented Jan 27, 2017

I like that this consolidates the any cast to a single location. The getIfPromise name seems all right. Other random ideas that cross my mind are maybePromise, or a safePromise wrapper which you would use like this:

safePromise(maybeAPromise).then(() => {
  // Will only be called if `maybeAPromise` had a `then` method...
});

(Costs: extra function creation, and attendant cost of capturing a closure.)

@leebyron
Copy link
Contributor Author

Yeah I'd like to avoid the costs of doing so, plus in a lot of those cases we're actually returning early if it's a promise, so that might not work.

I wish javascript supported if-let conditions, that would drop one LOC for each use.

This replaces the isThenable boolean check with a "getIfPromise" function. It's essentially the same, but rather than returning boolean, it's the identify function when it would have returned true, and returns void otherwise. That lets us give it a better type signature that Flow can do more with.

Open to feedback on this one, and open to bikeshedding on the name of this new function.
@leebyron leebyron merged commit d052f65 into master Jan 27, 2017
@leebyron leebyron deleted the thenable branch January 27, 2017 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants