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

Use native promises #53

Merged
merged 2 commits into from Sep 9, 2017
Merged

Use native promises #53

merged 2 commits into from Sep 9, 2017

Conversation

lbeschastny
Copy link
Contributor

No description provided.

@lbeschastny
Copy link
Contributor

Actually, cash already use native promises.

But it also use .promisify and .coroutine from bluebird package.
Probably you meant that cash should get rid of those, to which I agree.

The former could be replaced with a simple inline implementation, while the later could be replaced with a co package (only in master branch since next use async/await instead).

Right now no one is actively working on this project, but I'll be happy to review a PR.

@nickmccurdy
Copy link
Member Author

nickmccurdy commented Sep 6, 2017

Yeah, I meant to suggest dropping the dependency on bluebird. I've been contributing to Koala but I might start a PR for this soon if I have time.

@nickmccurdy
Copy link
Member Author

nickmccurdy commented Sep 7, 2017

I'm trying to replace Bluebird.coroutine on master. Replacing the function with co gives me tons of test failures, I can post them if you're interested. However if I replace Bluebird.coroutine(function * ...) with just function * ... the tests pass. Is this valid or do we still need a coroutine implementation here?

@lbeschastny
Copy link
Contributor

You've got a lot of errors because bluebirds's .coroutine does the same thing as co.wrap in co. So using just co(...) will break a lot of stuff.

@coveralls
Copy link

coveralls commented Sep 9, 2017

Coverage Status

Coverage remained the same at 95.161% when pulling 5ea6bfb on bluebird-cleanup into 4977334 on master.

@lbeschastny
Copy link
Contributor

Is this valid or do we still need a coroutine implementation here?

It's a very good observation.

Without co.wrap calling ctx.cashed will return a generator function instead of a promise, which is also yieldable. So, we don't really need any coroutibe wrapping here since koa already wraps everything with co.

@lbeschastny
Copy link
Contributor

I think I'll keep co.wrap, because removing it may break someone's code.

@lbeschastny lbeschastny merged commit accee53 into master Sep 9, 2017
@lbeschastny lbeschastny deleted the bluebird-cleanup branch September 9, 2017 18:26
@lbeschastny
Copy link
Contributor

@nickmccurdy just published bluebird-free versions to npm:

{ latest: '2.2.0', next: '3.1.0-0' }

@nickmccurdy
Copy link
Member Author

Perfect, thanks. I didn't think to try co.wrap.

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.

None yet

3 participants