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

Does not work in SystemJS and nodewebkit from 2.9.15 onwards #624

Closed
NervosaX opened this Issue May 19, 2015 · 10 comments

Comments

Projects
None yet
3 participants
@NervosaX

NervosaX commented May 19, 2015

Since upgrading from 2.9.14 to 2.9.15, bluebird throws an invalid invocation error when performing Promise.all() (possibly others, Promise.resolve() works fine). This is only an error when run through System JS and nwjs (nodewebkit). 2.9.15 seems to be when it adds the browser version of bluebird.

Loading it through nodewebkit with a direct CDN link seemed to work fine.

See this for reproduction steps: https://github.com/NervosaX/bluebird-nw-bug

The stack trace is:

TypeError: Illegal invocation
    at n.2.n._queueTick (https://npm.jspm.io/bluebird@2.9.15/js/browser/bluebird.js:2:2630)
    at n.2.n.invoke (https://npm.jspm.io/bluebird@2.9.15/js/browser/bluebird.js:2:2165)
    at e.23.e.exports.e._then (https://npm.jspm.io/bluebird@2.9.15/js/browser/bluebird.js:4:3618)
    at e.23.e.exports.e.then (https://npm.jspm.io/bluebird@2.9.15/js/browser/bluebird.js:4:1997)
    at file:///home/adam/Programming/js/bluebirdnw/dist/index.html:17:14
    at D (https://jspm.io/es6-module-loader@0.16.5.js:1:7439)
    at I (https://jspm.io/es6-module-loader@0.16.5.js:1:7071)
    at O.7.O.when (https://jspm.io/es6-module-loader@0.16.5.js:1:10745)
    at w.7.w.run (https://jspm.io/es6-module-loader@0.16.5.js:1:9781)
    at e.3.e._drain (https://jspm.io/es6-module-loader@0.16.5.js:1:1740)
    at 3.e.drain (https://jspm.io/es6-module-loader@0.16.5.js:1:1394)
    at process._tickCallback (node.js:375:11)

@benjamingr benjamingr added the bug label May 20, 2015

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr May 20, 2015

Collaborator

Nevermind, verified failure on 2.9.25

Collaborator

benjamingr commented May 20, 2015

Nevermind, verified failure on 2.9.25

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr May 20, 2015

Collaborator

This does not reproduce when doing <script src="./bluebird.js"></script> instead of using SystemJS in nw and also works correctly when using SystemJS in the browser http://jsfiddle.net/0voy1u7L/ , the suspect is detecting the scheduler mechanism.

This is the scheduler, setting Promise.setScheduler(function(fn){ setTimeout(fn, 0) }); fixes it

Update: this is because nw has a separate global.setImmediate that requires bounded invocation, doing setImmediate = global.setImmediate before importing bluebird fixes this.

We can work around this by binding setImmedaite or adding a check for typeof setImmediate !== undefined

Collaborator

benjamingr commented May 20, 2015

This does not reproduce when doing <script src="./bluebird.js"></script> instead of using SystemJS in nw and also works correctly when using SystemJS in the browser http://jsfiddle.net/0voy1u7L/ , the suspect is detecting the scheduler mechanism.

This is the scheduler, setting Promise.setScheduler(function(fn){ setTimeout(fn, 0) }); fixes it

Update: this is because nw has a separate global.setImmediate that requires bounded invocation, doing setImmediate = global.setImmediate before importing bluebird fixes this.

We can work around this by binding setImmedaite or adding a check for typeof setImmediate !== undefined

@NervosaX

This comment has been minimized.

Show comment
Hide comment
@NervosaX

NervosaX May 20, 2015

Wonderful, glad my ticket was thorough enough! Will use your workaround in the meantime.

NervosaX commented May 20, 2015

Wonderful, glad my ticket was thorough enough! Will use your workaround in the meantime.

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr May 25, 2015

Collaborator

@NervosaX thanks for the detailed issue - can you please verify it worked for you?

Collaborator

benjamingr commented May 25, 2015

@NervosaX thanks for the detailed issue - can you please verify it worked for you?

@NervosaX

This comment has been minimized.

Show comment
Hide comment
@NervosaX

NervosaX May 25, 2015

@benjamingr
I seem to be getting the following error now:

Uncaught TypeError: undefined is not a functionbluebird.js:3422 31.schedulebluebird.js:248   

2.Async._queueTickbluebird.js:186 AsyncSettlePromisesbluebird.js:2486   

23.module.exports.Promise._queueSettlePromisesbluebird.js:2499  

23.module.exports.Promise._fulfillUncheckedbluebird.js:2422   

23.module.exports.Promise._fulfillbluebird.js:2325   

23.module.exports.Promise._resolveCallbackbluebird.js:3729 resolveFromThenable  

Which is here:

          var version = process.versions.node.split(".").map(Number);
          schedule = (version[0] === 0 && version[1] > 10) || (version[0] > 0) ? function(fn) {
            global.setImmediate(fn); <-------------
          } : process.nextTick;

It is complaining that global.setImmediate does not exist. There is global.setTimeout. I would say this is an incompatability with node webkit now.

NervosaX commented May 25, 2015

@benjamingr
I seem to be getting the following error now:

Uncaught TypeError: undefined is not a functionbluebird.js:3422 31.schedulebluebird.js:248   

2.Async._queueTickbluebird.js:186 AsyncSettlePromisesbluebird.js:2486   

23.module.exports.Promise._queueSettlePromisesbluebird.js:2499  

23.module.exports.Promise._fulfillUncheckedbluebird.js:2422   

23.module.exports.Promise._fulfillbluebird.js:2325   

23.module.exports.Promise._resolveCallbackbluebird.js:3729 resolveFromThenable  

Which is here:

          var version = process.versions.node.split(".").map(Number);
          schedule = (version[0] === 0 && version[1] > 10) || (version[0] > 0) ? function(fn) {
            global.setImmediate(fn); <-------------
          } : process.nextTick;

It is complaining that global.setImmediate does not exist. There is global.setTimeout. I would say this is an incompatability with node webkit now.

@NervosaX

This comment has been minimized.

Show comment
Hide comment
@NervosaX

NervosaX May 26, 2015

Could possible be related to #525

Version 2.9.25 and your fix above works, but 2.9.26 fails with the above issue.

NervosaX commented May 26, 2015

Could possible be related to #525

Version 2.9.25 and your fix above works, but 2.9.26 fails with the above issue.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov
Owner

petkaantonov commented May 26, 2015

@benjamingr pls fix

@benjamingr

This comment has been minimized.

Show comment
Hide comment
@benjamingr

benjamingr May 26, 2015

Collaborator

I'll look into it tomorrow, I think we can just explicitly detect node webkit and use process.nextTick there.

Collaborator

benjamingr commented May 26, 2015

I'll look into it tomorrow, I think we can just explicitly detect node webkit and use process.nextTick there.

@petkaantonov

This comment has been minimized.

Show comment
Hide comment
@petkaantonov

petkaantonov May 30, 2015

Owner

@NervosaX does it work for you in 2.9.27? I added capability to run tests in nw and they pass. It also changed scheduler to be used in nw to MutationObserver since it's faster than setImmediate and not as buggy as nextTick.

Owner

petkaantonov commented May 30, 2015

@NervosaX does it work for you in 2.9.27? I added capability to run tests in nw and they pass. It also changed scheduler to be used in nw to MutationObserver since it's faster than setImmediate and not as buggy as nextTick.

@NervosaX

This comment has been minimized.

Show comment
Hide comment
@NervosaX

NervosaX Jun 1, 2015

@petkaantonov Seems to be working as normal in 2.9.27, both in browser and through nw. Thanks heaps for the fix.

NervosaX commented Jun 1, 2015

@petkaantonov Seems to be working as normal in 2.9.27, both in browser and through nw. Thanks heaps for the fix.

@NervosaX NervosaX closed this Jun 1, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment