1

I have this piece of code that uses promises API to check if a the buffering function from my serviceProvider component has completed. The serviceProvider has a flag for that, which can be polled with getBufferingStatus().

When using ensureBufferingHasFinished().then(....).catch(err => console.log(err)), it shows me that the condition cannot be accessed from this.serviceProvider

I have used promises before, but this is the first time I am using one in this particular situation. How can I bind this. to the promise? All help is greatly appreciated!

ensureBufferingHasFinished() {
  return new Promise(function (resolve, reject) {
    (function waitForBufferingComplete(){
        if (!this.serviceProvider.getBufferingStatus()) {
          alert("RESOLVED");
          return resolve();
        }
        setTimeout(waitForBufferingComplete, 250);
    })();
  });
}
El Fred
  • 330
  • 1
  • 7
  • 23
  • 1
    To avoid wasting time solving such problems, always prefer using `Fat Arrow` functions – Anand Undavia Aug 06 '18 at 16:32
  • pass in `this` to `ensureBufferingHasFinished`. – Jimenemex Aug 06 '18 at 16:34
  • You are in the callback you passed to `new Promise()` which sets a new value for `this`. If you use a fat arrow callback declaration instead of your normal `function` declaration it will preserve the value of `this`. – jfriend00 Aug 06 '18 at 16:54
  • @AnandUndavia - It is wrong to suggest that one always use fat arrow functions. They have a useful place and a place where they should not be used. One needs to know when to use them and when not to in order to program correctly. For example, declaring a method as a fat arrow function will screw up the value of `this`. – jfriend00 Aug 06 '18 at 16:55
  • @jfriend00 Ohh! Can you share some examples or point me to some online resource which discusses about where you would prefer using `function` over fat arrow ones. Thanks :D – Anand Undavia Aug 06 '18 at 17:00
  • 1
    @AnandUndavia - I did share one example already in my previous comment (method definitions). Any callback that sets the `this` value to the desired value and want to use that value (such as DOM event listener in the browser). Basically any function where some outside agent is going to set `this` for you should not be a fat arrow function because that will override the desired value of `this`. – jfriend00 Aug 06 '18 at 17:06
  • @jfriend00 makes sense! Thank you :D – Anand Undavia Aug 06 '18 at 17:08
  • What horrible `serviceProvider` is this that needs to be polled instead of providing a callback API? – Bergi Aug 06 '18 at 17:28
  • One that I created myself. I am just a hobby coder :) – El Fred Aug 06 '18 at 17:31
  • @ElFred You might want to fix that instead of making such a complicated promise :-) – Bergi Aug 06 '18 at 17:33
  • @Bergi, I was waiting for someone to point it out. Would you have a good guide for a noob like me? – El Fred Aug 06 '18 at 17:35

5 Answers5

3

The this keyword is a source of headaches to people unfamiliar with the nuances of how JavaScript assigns its value. The value of this corresponds to execution context, and not scope. In this case, waitForBufferingComplete's execution context isn't the same as the context ensureBufferingHasFinished is called with.

You have a few options on how to ensure you're accessing what you need.

Assign variables to outer scope

The age-old hack of assigning this or properties thereof is quick and reliable in situations like yours where functions are nested within each other:

function ensureBufferingHasFinished() {
    var that = this,
        sp = this.serviceProvider;
    return new Promise(function (resolve, reject) {
        (function waitForBufferingComplete() {
            //you can use "that.serviceProvider", or "sp"
            if (!sp.getBufferingStatus()) {
                alert("RESOLVED");
                return resolve();
            }
            setTimeout(waitForBufferingComplete, 250);
        })();
    });
}

Use function.bind()

Calling bind on a function forces it to have the execution context you explicitly give as its argument, and is more useful if you do not or cannot have your callbacks within the scope containing the value you need:

function ensureBufferingHasFinished() {
    return new Promise(function (resolve, reject) {
        (function waitForBufferingComplete() {
            if (!this.serviceProvider.getBufferingStatus()) {
                alert("RESOLVED");
                return resolve();
            }
            setTimeout(waitForBufferingComplete.bind(this), 250);
        }.bind(this))();
    }.bind(this));
}

or

function ensureBufferingHasFinished() {
    return new Promise(_resolveBufferingPromise.bind(this));
}

function _resolveBufferingPromise(resolve, reject) {
    (function waitForBufferingComplete() {
        if (!this.serviceProvider.getBufferingStatus()) {
            alert("RESOLVED");
            return resolve();
        }
        setTimeout(waitForBufferingComplete.bind(this), 250);
    }.bind(this))();
}

You can also pass the serviceProvider to the IIFE you made around waitForBufferingComplete, though with your code structure you should only do this if you're supporting ES5-compatible browsers as setTimeout didn't support passing additional parameters until then:

function ensureBufferingHasFinished() {
    return new Promise(function (resolve, reject) {
        (function waitForBufferingComplete(serviceProvider) {
            if (!serviceProvider.getBufferingStatus()) {
                alert("RESOLVED");
                return resolve();
            }
            setTimeout(waitForBufferingComplete, 250, serviceProvider);
        })(this.serviceProvider);
    }.bind(this));
}

Use arrow functions (ES2015 or later)

If you're developing for platforms that support ES2015, that version introduced arrow functions which ignore execution context and retain the lexical scope of this from its parent:

function ensureBufferingHasFinished() {
    return new Promise((resolve, reject) => {
        var waitForBufferingComplete = () => {
            if (!this.serviceProvider.getBufferingStatus()) {
                alert("RESOLVED");
                return resolve();
            }
            setTimeout(waitForBufferingComplete, 250);
        }
    });
}

Read up on more about this on MDN.

p0lar_bear
  • 2,203
  • 2
  • 21
  • 31
  • Many working proposals in this post. But in terms of performance, which is the one to be preferred? – El Fred Aug 06 '18 at 17:03
  • It's less about performance and more about keeping your code readable, being consistent to avoid future confusions like this, or adhering to any standards imposed upon you by a team. Assigning `this` or its properties to a variable or passing it down the chain has negligible impact on performance and memory usage because you're passing a *reference*, not values. For example, while `function.bind()` is a valid way to do this, I prefer the first or third methods as I find it's easier to forget to add your `bind` calls to callbacks, and it doesn't look as elegant. – p0lar_bear Aug 06 '18 at 17:08
  • In addition, you obviously cannot use arrow functions if you're targeting ES3 or ES5 (e.g. Internet Explorer 9 or earlier, or older versions of Firefox/Chrome). – p0lar_bear Aug 06 '18 at 17:10
  • 1
    You must also `bind` the `waitForBufferingComplete` that you pass to `setTimeout` – Bergi Aug 06 '18 at 17:30
  • @Bergi good catch, I've edited my answer to reflect that. – p0lar_bear Aug 06 '18 at 17:37
1

What can you do is to pass the serviceProvider instance in the arguments of ensureBufferingHasFinished.

ensureBufferingHasFinished(serviceProvider) {
 return new Promise(function (resolve, reject) {
  (function waitForBufferingComplete(){
       if (!serviceProvider.getBufferingStatus()) {
         alert("RESOLVED");
         return resolve();
       }
       setTimeout(waitForBufferingComplete, 250);
   })();
 });
}
kaosdev
  • 269
  • 1
  • 7
1

Bind this to Promise function and pass the serviceProvider as a parameter to the iife. Pass the serviceProvider as the third parameter to the setTimeout

ensureBufferingHasFinished() {
  return new Promise(function (resolve, reject) {
    (function waitForBufferingComplete(serviceProvider){
        if (!serviceProvider.getBufferingStatus()) {
          alert("RESOLVED");
          return resolve();
        }
        setTimeout(waitForBufferingComplete, 250, serviceProvider);
    })(this.serviceProvider);
  }.bind(this));
}
Lasithe
  • 1,916
  • 1
  • 15
  • 20
1

Your this is pointing to the inner scope of the function. You can use a local variable to store the reference of this and use it in all inner scopes:

ensureBufferingHasFinished() {
let ref = this;
return new Promise(function (resolve, reject) {
    (function waitForBufferingComplete(){
        if (!ref.serviceProvider.getBufferingStatus()) {
          alert("RESOLVED");
          return resolve();
        }
        setTimeout(waitForBufferingComplete, 250);
    })();
  });
}
Sandeep Kumar
  • 2,397
  • 5
  • 30
  • 37
-1

Try this:

    waitForBufferingComplete = function(bufferingSuccess){
            if (!this.serviceProvider.getBufferingStatus()) {
                  bufferingSuccess(true);
            }
            else{
                setTimeout(waitForBufferingComplete, 250);
            }
        }

        ensureBufferingHasFinished() {
          return new Promise(function (resolve, reject) {
            waitForBufferingComplete(function(isFinishedBuffering){
                if (isFinishedBuffering){
                    resolve('ok');
                }                    
            });
          });
        }
quangho
  • 61
  • 1
  • 1
  • 6