Skip to content

this scoping issues with setTimeout #97

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

Open
rafkhan opened this issue Jul 2, 2017 · 3 comments
Open

this scoping issues with setTimeout #97

rafkhan opened this issue Jul 2, 2017 · 3 comments

Comments

@rafkhan
Copy link

rafkhan commented Jul 2, 2017

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)
@lynnaloo
Copy link
Collaborator

We love pull requests!

@rwaldron
Copy link
Contributor

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.

@rafkhan
Copy link
Author

rafkhan commented Sep 22, 2017

@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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants