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

Provide compiler warnings/errors when using method as callback and this context won't be preserved #26881

Closed
4 tasks done
craxal opened this issue Sep 4, 2018 · 2 comments
Closed
4 tasks done
Labels
Duplicate An existing issue was already created

Comments

@craxal
Copy link

craxal commented Sep 4, 2018

Search Terms

bind, this, callback

Suggestion

Emit a warning or error when a method meets these conditions:

  • The method is implemented using standard method syntax.
  • The method needs this context to be preserved (uses the this keyword).
  • The method is passed as a callback to another function.

Use Cases

The standard syntax for defining methods is clean, concise, familiar, and (arguably) preferable to everyone. However, methods defined using this common syntax will not preserve the context of this when passed as a callback to another function. This mistake is easy to make, yet perfectly legal!

class Example {
    private _privateVar: any = "Hello World!";

    /**
     * Method syntax is the standard syntax we all know and love.
     * The problem is `this` context won't be preserved if used as a callback.
     */
    public methodSyntax(): any {
        return this._privateVar;
    }

    /**
     * Instance syntax solves the `this` problem, but it comes with other drawbacks/trade offs.
     */
    public instanceSyntax = () => {
        return this._privateVar;
    }
}

function doSomething(func: Function): any {
  return func();
}

let obj = new Example();
let result1 = doSomething(obj.methodSyntax); // undefined (OOPS!)
let result2 = doSomething(() => obj.methodSyntax()); // "Hello World!"
let result3 = doSomething(obj.methodSyntax.bind(obj)); // "Hello World!"
let result4 = doSomething(obj.instanceSyntax); // "Hello World!"

Examples

Using the above example, I'd love to see the compiler offer the suggestions made in this page. Something like this

let obj = new Example();

// Error: Method using `this` context should not be used as callback.
//  Consider:
//  - Refactoring the method as an instance method.
//  - Surrounding the method argument with a local anonymous function.
//  - Using the `bind` function.
let result1 = doSomething(obj.methodSyntax);

let result2 = doSomething(() => obj.methodSyntax()); // OK
let result3 = doSomething(obj.methodSyntax.bind(obj)); // OK
let result4 = doSomething(obj.instanceSyntax); // OK

Checklist

My suggestion meets these guidelines:

  • This wouldn't be a breaking change in existing TypeScript / JavaScript code
  • This wouldn't change the runtime behavior of existing JavaScript code
  • This could be implemented without emitting different JS based on the types of the expressions
  • This isn't a runtime feature (e.g. new expression-level syntax)
@ghost
Copy link

ghost commented Sep 4, 2018

Duplicate of #10288

@ghost ghost marked this as a duplicate of #10288 Sep 4, 2018
@ghost ghost added the Duplicate An existing issue was already created label Sep 4, 2018
@typescript-bot
Copy link
Collaborator

This issue has been marked as a 'Duplicate' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants