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

Question: Why does "this" not have a type in function assigned to class prototype #8024

Closed
ToddThomson opened this issue Apr 12, 2016 · 10 comments
Assignees
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light

Comments

@ToddThomson
Copy link
Contributor

TypeScript Version:

1.8.9

Code

export class Configuration {
    private storage: string;

    constructor() {
        this.storage = "InMemoryStorageProvider()";
    }

    public useLocalStorage(): void {
        // This method will be injected via the prototype.
    }    
}
import { Configuration } from './classes/Configuration';

Configuration.prototype.useLocalStorage = function() {
    this.storage = "NodeFileStorageProvider"
};

Expected behavior:

When walking the AST I use:

let identifier: ts.Identifier = <ts.Identifier>node;
let identifierSymbol: ts.Symbol = this.checker.getSymbolAtLocation( identifier );

to obtain the identifier and its possible associated symbol when the node is SyntaxKind.Identifier.

for the first 2 references to the private property storage, I obtain the identifier and the symbol (which correctly have the same Id). With the prototype function assigned to useLocalStorage the reference to storage the call to this.checker.getSymbolAtLocation( identifier ) does not return a symbol.

Actual behavior:

I am using the AST to identify identifiers which may be minified/shortened. The 1st and 2nd reference to storage gets shortened as it is a private property.

My expectation was that the reference to storage would have the same symbol ( not undefined ) as the other references to the storage property.

@ToddThomson ToddThomson changed the title Reference to Identifier in function assigned to class prototype no symbol Reference to Identifier in function assigned to class prototype has no symbol Apr 12, 2016
@ToddThomson
Copy link
Contributor Author

Additionally, the resolution of the Type for this in:

Configuration.prototype.useLocalStorage = function() {
    this.storage = "NodeFileStorageProvider"
};

results in Any. This was not what I was expecting.

@ToddThomson
Copy link
Contributor Author

Not sure why the type of this cannot be determined as in function checkThisExpression().

Code fragment to use:

                // If this is a function in a JS file, it might be a class method. Check if it's the RHS
                // of a x.prototype.y = function [name]() { .... }
                if (container.kind === SyntaxKind.FunctionExpression) {
                    if (getSpecialPropertyAssignmentKind(container.parent) === SpecialPropertyAssignmentKind.PrototypeProperty) {
                        // Get the 'x' of 'x.prototype.y = f' (here, 'f' is 'container')
                        const className = (((container.parent as BinaryExpression)   // x.prototype.y = f
                            .left as PropertyAccessExpression)       // x.prototype.y
                            .expression as PropertyAccessExpression) // x.prototype
                            .expression;                             // x
                        const classSymbol = checkExpression(className).symbol;
                        if (classSymbol && classSymbol.members && (classSymbol.flags & SymbolFlags.Function)) {
                            return getInferredClassType(classSymbol);
                        }
                    }
                }

@ToddThomson ToddThomson changed the title Reference to Identifier in function assigned to class prototype has no symbol Question: Why does "this" not have a type in function assigned to class prototype Apr 15, 2016
@ToddThomson
Copy link
Contributor Author

ToddThomson commented Apr 15, 2016

OK. I understand that this is resolved to the any Type when referenced within a function that has been assigned to the class prototype. This is the reason why there is no symbol for the identifier referenced by this.indentifierName. However, I am not clear why the Type cannot be resolved to the class type.
In any event, I've written the code so that I can get the symbol of the identifier and thus shorten the identifier name occurrence, but I am curious as to the reason that TypeScript does not resolve the type of this to the class type?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 15, 2016

I do not think there is really a good reason why it is the way it is. It is definitely possible to flow the this type as we flow the contextual type. i will bring this to discussion.

@mhegazy mhegazy added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Apr 15, 2016
@sandersn
Copy link
Member

In the meantime, the workaround is to give this for the method that's intended to be filled in later:

class Configuration {
  // ...
  public useLocalStorage(this: this): void {
  }
}

Then contextual typing will pick up the type of this in the function you assign to Configuration.prototype.useLocalStorage.

@ToddThomson
Copy link
Contributor Author

ToddThomson commented Apr 16, 2016

@sandersn Thank-you. My context here is walking the AST to find identifiers to minify. In this particular case a user had an issue with his code ( this gist of it is above ) not being minified properly. As @mhegazy said, there is no reason why the type of this cannot be resolved in this context so I added that capability to my code.

@mhegazy mhegazy added Committed The team has roadmapped this issue Help Wanted You can do this Good First Issue Well scoped, documented and has the green light and removed In Discussion Not yet reached consensus labels May 17, 2016
@mhegazy mhegazy added this to the TypeScript 2.1 milestone May 17, 2016
@mhegazy
Copy link
Contributor

mhegazy commented May 17, 2016

PRs are welcomed.

@mhegazy mhegazy modified the milestones: Future, TypeScript 2.1 Sep 21, 2016
@niemyjski
Copy link

Was any progress made on this?

@sandersn
Copy link
Member

In 2.3 with --noImplicitThis, assigning a function to a prototype property should give this a contextual type, so

Configuration.prototype.useLocalStorage = function() {
    this.storage = "NodeFileStorageProvider"
};

Should give the type Configuration to this in the body of the function.

You can also use the new --strict flag, which includes --noImplicitThis.

@sandersn
Copy link
Member

Fixed by #14141

@sandersn sandersn added Fixed A PR has been merged for this issue and removed Help Wanted You can do this Suggestion An idea for TypeScript labels Apr 12, 2017
@mhegazy mhegazy removed this from the Future milestone Apr 26, 2018
@mhegazy mhegazy added this to the TypeScript 2.4 milestone Apr 26, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Committed The team has roadmapped this issue Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light
Projects
None yet
Development

No branches or pull requests

4 participants