-
Notifications
You must be signed in to change notification settings - Fork 133
Use native promises before others #61
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
Comments
Thank you for the suggestion. The downside of using native promises seems to be that a user would get back a different type of Promise depending on how old the browser was, rather than if they chose to include jQuery in their project, and have to do additional detection on their part. What if the constructor took an optional argument that disables the autodetection (like |
Yeah I actually thought of this but forgot to mention it. I assume the priority is to be backwards compatible. So an option like that is probably the best idea. Alternatively something like this:
Similar to this: octokit/octokit.js#262 That way you can supply any promise lib you want as long as it is ES 2015 (still want to call it ES6 :)) compatible. But for me any approach will do. |
Thanks for the feedback! Sorry it took so long to respond. #81 splits octokat into smaller pieces so users can opt-in to different features (like I hope that helps address this annoying "feature" 😄 . |
(Apologies for the late reply) Octokat now uses native promises before 3rd-party libraries (and will likely drop support for those soon). Please reopen this issue if you still have a problem. |
Now that ES 2015 is final it would be better to use native promises before any external libraries. https://github.com/philschatz/octokat.js/blob/master/src/helper-promise.coffee#L17
I happened to be using jquery promises without knowing it.
The following also didn't chain properly, it resolved with a promise not the values.. not surprising as jquery promises are very broken. It also does not allow me to use
catch
. Not very nice since that is a standard method on a promise now.I can either wrap this in a proper promise implementation (I use bluebird) or works if I manually edit the octokat code to not use jquery.
The text was updated successfully, but these errors were encountered: