Skip to content
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

Track type of this in callbacks #968

Closed
danvk opened this issue Oct 21, 2015 · 2 comments
Closed

Track type of this in callbacks #968

danvk opened this issue Oct 21, 2015 · 2 comments

Comments

@danvk
Copy link
Contributor

danvk commented Oct 21, 2015

There's an error (marked with <---) in this snippet which Flow could potentially catch:

class RemoteFile {
  url: string;
...
  promiseXHR(xhr: XMLHttpRequest): Q.Promise<[any, Event]> {
    var url = this.url;
    var deferred = Q.defer();
    xhr.addEventListener('load', function(e) {
      if (this.status >= 400) {
        deferred.reject(this.status + ' ' + this.statusText);
      } else {
        deferred.resolve([this.response, e]);
      }
    });
    xhr.addEventListener('error', function(e) {
      deferred.reject(`Request for ${this.url} failed: ${this.status}`);  // <---
    });
    this.numNetworkRequests++;
    xhr.send();
    return deferred.promise;
  }
}

The problem is that this inside the addEventListener callback refers to the XMLHttpRequest object (I believe) and not the RemoteFile class. The latter has a url property but not a status property, whereas the former has status but not url. Clearly this code isn't right!

Flow doesn't complain, though. It thinks this has type any in the event listener.

Where possible, the DOM declarations should indicate the type of this in callbacks to prevent this sort of mistake.

For example, Google's Closure Compiler provides an @this annotation. You can see an example of this in action in the declaration for Array.prototype.find.

@samwgoldman
Copy link
Member

Ref #452

@danvk
Copy link
Contributor Author

danvk commented Oct 22, 2015

Closing as a dupe

@danvk danvk closed this as completed Oct 22, 2015
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

2 participants