taylorhakes/promise-polyfill

Different behavior between Chrome/Firefox implementation and polyfill

Closed this issue · 10 comments

I'm having a hard time nailing it down, but my code works in both the latest Firefox and Chrome, and when I run the tests in Selenium, but when I require the polyfill for Poltergeist/Phantomjs, I get different behavior. It appears that my callback isn't being called when I nest promises. Pardon the messy code, it's in transition:

var TimeframeStore = {
  ...
  getAll: function () {
    return new Promise(function (resolve, reject) {
      TaskStore.getAll().then(function (data) {
        this.updateModels(data);
        request({
          method: 'get',
          url: this.url(),
          success: function (data) {
            medianProductivity = data.meta.medianProductivity;
            resolve(this.getData());
          }.bind(this)
        });
      }.bind(this));
    }.bind(this));
  }
}

and inside my component:

var TimeframesIndex = React.createClass({
  ...
  componentDidMount: function () {
    TimeframeStore.getAll().then(function (data) {
      this.updateTimeframes(data);
      TimeframeStore.on('change', this.loadTasks);
    }.bind(this));
  },

  loadTasks: function () {
    TimeframeStore.getAll().then(this.updateTimeframes);
  },
  ...
}

It works as expected in componentDidMount, but when it gets called a second time in loadTasks, even though I can confirm that resolve gets called within the nested promise, updateTimeframes never gets called.

This might be an issue with the way you are polyfilling, but I can't be sure. Can you make sure there isn't another error? Try putting an error handler on your getAll() call. The error should tell you what is going on.

Right now I'm polyfilling like:

if (!window.Promise) {
  window.Promise = require('promise-polyfill');
}

It works fine in other situations. I just tried adding error handlers both to loadTasks and as a second argument to TaskStore.getAll() and saw nothing from either of them. If I log out in loadTasks and in the success: callback from request it shows up for both, but not in updateTimeframes after that. I may just sidestep the issue by getting rid of the second request. It was a quick and dirty attempt to tie the stores together, but I think I know where I want it to go next.

I haven't seen an issue like this before. I and others are using the library the exact same way in production. Not saying there isn't an issue, but most likely it is something in your model code. Is the code in a public repo? I'll take a look. If not, it would be really helpful if you can reproduce the issue in jsfiddle.

Since you are only seeing the issue when polyfilling, maybe some code is executing before you do

if (!window.Promise) {
  window.Promise = require('promise-polyfill');
}

? Maybe code is holding onto an undefined reference? Just a guess. I might be way off.

It's a private rails app, but I can add you as a contributor if you want to poke around. There isn't an existing reference, since when I remove the polyfill it throws an error saying Promise is not defined or the like, which is what prompted me to add it in the first place. I'm not too sure how to go about breaking it down into a more useful repro, but I'll take another look at it tomorrow morning and see what I can come up with.

Another thought. Could you try testing another library? Try using bluebird or another Promise library. Do you still see the issue?

Sure, I'll give that a shot.

Hmm, fails the same way with bluebird, too. It also fails Selenium if I override the browser implementation with the polyfill, though I haven't been able to reproduce the issue manually in the browser.

I am not too familiar with debugging PhantomJS. I know there are some ways to open the Chrome debugger and see what is going on. http://phantomjs.org/troubleshooting.html . The other option is to throw some console.log statements in your code. Let me know what you find.

Promises are pretty basic at their core. It keeps an array of callbacks and then calls each callback in the array asynchronously when resolve or reject is called.

Here is a link to where the library calls the callbacks async https://github.com/taylorhakes/promise-polyfill/blob/master/Promise.js#L33 . Might help to throw a console log statement in there or pause the debugger and see what is happening.

Okay, I think I know what's wrong. I'm using sinon.useFakeTimers() in my tests which stubs out setTimeout. handle gets called, but the scheduled function in asap never does. I'm guessing the native implementation doesn't make use of setTimeout, which is why it works with Selenium. And the fact that I can't reproduce the error in dev is because I'm not using sinon.

I'll go ahead and close this, but if you have any suggestions I'd be glad to hear them. It would be great if there was a way to set the browser time without stopping it completely. I tried using clock.tick(5) and that didn't make a difference...

I've gone with a different approach which allows me to go without sinon.useFakeTimers(), but I just came across this issue on Bluebird that seems like it deals with this use case. Not sure if it's something that promise-polyfill should address as well: petkaantonov/bluebird#631