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

Replace Future#promise() with Promises/A+ compatible Future#then() #35

Closed
wants to merge 1 commit into from

Conversation

Avaq
Copy link
Member

@Avaq Avaq commented Oct 20, 2016

So this might be an interesting topic for conversation. Using the new CachedFuture class I created while working on #34, I was able to create a fully Promises/A+ compliant then method. Essentially turning Fluture into a Promise library! o.0

I think the library might benefit from having this. Particularly because fork is not standardized. Though I'm very eager to hear peoples opinions. Check out the diff in the readme to learn about how I advice the feature to be used.

@codecov-io
Copy link

codecov-io commented Oct 20, 2016

Current coverage is 100% (diff: 100%)

No coverage report found for master at 6103927.

Powered by Codecov. Last update 6103927...c530aae

@Avaq
Copy link
Member Author

Avaq commented Oct 21, 2016

An alternative is to provide then as such:

function(onResolve, onReject){
  return new Promise((res, rej) => this._f(rej, res)).then(onResolve, onReject);
}

And not concern myself with the complexity of Promises. The intended use for then would remain the same. The only difference being that you won't be able to use Future methods on the return value - perhaps even for the best.

@davidchambers
Copy link
Contributor

I'm concerned that this may confuse new users of the library.

@Avaq
Copy link
Member Author

Avaq commented Oct 24, 2016

I think it's quite cool that Futures can also be Promises. But that's where I run out of pro's. I think making Futures Promises enables a bunch of bad practices and nothing more. I believe providing a then function which returns a Promise has less pitfalls, but offers nothing more than a magical convenience over .promise(). I'm sure this PR will enjoy its new life as reference material. ;)

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

3 participants