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 global Promise #38

Closed
wants to merge 1 commit into from
Closed

Use global Promise #38

wants to merge 1 commit into from

Conversation

statianzo
Copy link

Expect a globally available Promise either natively available or
provided through a polyfill.

.always is removed from the promise api as it's non-standard. Related
tests are removed.

Given that resolve and reject only accept one argument, responses
are passed through as {data: parsedResponse, xhr: xhr}. Errors
provided to reject have the same interface.

Added es6-promise as a devDependency because PhantomJS doesn't have
native promises yet.

Opening this as a discussion point for #20.

Expect a globally available Promise either natively available or
provided through a polyfill.

`.always` is removed from the promise api as it's non-standard. Related
tests are removed.

Given that `resolve` and `reject` only accept one argument, responses
are passed through as `{data: parsedResponse, xhr: xhr}`. Errors
provided to `reject` have the same interface.

Added `es6-promise` as a devDependency because PhantomJS doesn't have
native promises yet.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 2292b91 on statianzo:promises into f5c6d94 on fdaciuk:dev.

@fdaciuk
Copy link
Owner

fdaciuk commented Aug 9, 2016

Nice work @statianzo! Thanks ✨
I'll try it today, before merge.

Thanks again =)

@fdaciuk fdaciuk added this to the v3.0.0 milestone Aug 9, 2016
@statianzo
Copy link
Author

statianzo commented Aug 9, 2016

I'm still not certain on the shape of the response object. Looked at the visionmedia/superagent Response object and they have a few convenience things.

{
  xhr: xhr,
  req: req,
  text: xhr.responseText,
  body: parsedBody,
  status: 200,
  statusText: 'OK',
  //...
}

Also, if you plan on merging, I'd be fine with updating the docs first.

@fdaciuk
Copy link
Owner

fdaciuk commented Aug 15, 2016

Yes, we need to think about the returned object.
Sorry, but I'm really busy this week. I'll try to review this PR in the next week, ok?
Thanks for all job, for now =)

@fdaciuk
Copy link
Owner

fdaciuk commented May 21, 2017

Hey @statianzo! Sorry for a so long delay =)
I'm working on a ES6 version for this module, and I'll use Promise on that.. This way we'll can use both: generated ES5 version for simple projects, and ES6 version when we use webpack or something like that to compile =)

Thanks for suggestion and all work on this! ❤️

@fdaciuk fdaciuk closed this May 21, 2017
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