-
Notifications
You must be signed in to change notification settings - Fork 20
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
Remove es6-promise dependency #88
Conversation
…ion to be defined as an option, defaulting to the global Promise
@@ -36,10 +36,15 @@ Cherrytree.prototype.initialize = function (options) { | |||
this.middleware = [] | |||
this.options = _.extend({ | |||
interceptLinks: true, | |||
logError: true | |||
logError: true, | |||
Promise: window.Promise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to do smth like typeof window === 'undefined' ? global.Promise : window.Promise
to continue supporting node env.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Specifically the node env support is just during webpack right? Then global.Promise
should be fine. Also the actual webpack command should run in CI to catch this :)
Will try to sort out those travis builds a bit later. |
There's nothing to sort out unfortunately. Travis does not export secure env vars on pull requests for security reasons, so PR builds will always fail. |
I added my own saucelabs credentials on my fork and you can see the build passing |
@@ -36,10 +36,13 @@ Cherrytree.prototype.initialize = function (options) { | |||
this.middleware = [] | |||
this.options = _.extend({ | |||
interceptLinks: true, | |||
logError: true | |||
logError: true, | |||
Promise: global.Promise |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
neat, I didn't realise this would work in all envs (browser and node) with the help of webpack
Remove es6-promise dependency
Cool thanks! Didn't know about |
A promise implementation must be specified as an option now if no global Promise implementation exists.