node-rolling-spider icon indicating copy to clipboard operation
node-rolling-spider copied to clipboard

`this` scoping issues with `setTimeout`

Open rafkhan opened this issue 7 years ago • 3 comments

It appears as though the line setTimeout(rollingSpider.land, 5000); fails with an undefined error because setTimeout is setting this to the global scope. I'd be willing to contribute a fix if it's not being worked on. (Solution here)

// WORKS
setTimeout(function() { rollingSpider.land(); }, 5000);

// BREAKS 
setTimeout(rollingSpider.land, 5000);

...

/Users/rafy/Developer/drone-js-1/node_modules/rolling-spider/lib/drone.js:584
  this.logger('RollingSpider#land');
       ^

TypeError: this.logger is not a function
    at Timeout.land [as _onTimeout] (/Users/rafy/Developer/drone-js-1/node_modules/rolling-spider/lib/drone.js:584:8)
    at ontimeout (timers.js:365:14)
    at tryOnTimeout (timers.js:237:5)
    at Timer.listOnTimeout (timers.js:207:5)

rafkhan avatar Jul 02 '17 21:07 rafkhan

We love pull requests!

lynnaloo avatar Sep 22 '17 20:09 lynnaloo

setTimeout(rollingSpider.land, 5000);

I can't actually find this code anywhere in the project 0_o


The land() method of a RollingSpider instance is this sensitive and cannot be called as a an argument to setTimeout without having its context explicitly bound. Two solutions, which require no library code or example program changes:

let land = rollingSpider.land.bind(rollingSpider);
setTimeout(land, 5000);
setTimeout(() => rollingSpider.land(), 5000);

The second is likely the best.

rwaldron avatar Sep 22 '17 20:09 rwaldron

@rwaldron I ended up using the first pattern back when I opened this issue. I wanted to reuse the reference to land, so the first was definitely easier. I took a stab at changing the library so I wouldn't have to bind every function I wanted to use. When I'm at my personal computer again, I can try and dig it up, dust it off and send a PR :)

rafkhan avatar Sep 22 '17 20:09 rafkhan