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

Consider contextually typing class member functions by their base class/interface members #1373

Closed
kns98 opened this issue Dec 4, 2014 · 11 comments
Assignees
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Suggestion An idea for TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it

Comments

@kns98
Copy link

kns98 commented Dec 4, 2014

here is my code

    export enum SortDir
    {
        Asc,
        Desc,
        Flip
    }

    export interface ISortInfo
    {
        getvalue(x: QuerySummary): any; //change to number|string in TypeScript 1.4
        order: SortDir;
        ordercalc?: number;
    }

    class SortByQueryId implements ISortInfo
    {
        getvalue = x => x.QueryId;
        order = SortDir.Flip;
    }

when I type "x." I expect to see the properties of QuerySummary but I do not. VS still thinks the type is 'any'

@danquirk
Copy link
Member

danquirk commented Dec 5, 2014

Contextual typing has only ever been applied to expressions and not statements so this has never worked. That said, it's certainly a reasonable thing to expect, at least for the function declaration style:

class SortByQueryId implements ISortInfo {
    getvalue(x) { /* x should be QuerySummary in here */ }
}

Edited the title slightly to reflect this.

@danquirk danquirk changed the title visual studio not showing context for lamba function Consider contextually typing class member functions by their base class/interface members Dec 5, 2014
@danquirk danquirk added Suggestion An idea for TypeScript Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Dec 5, 2014
@kns98
Copy link
Author

kns98 commented Dec 5, 2014

Thanks Dan. I love this open community style development process :)

@jbrantly
Copy link

jbrantly commented Feb 3, 2015

Can the scope of this be expanded to support member properties and static properties? See my comment in #1887 for an example.

@hdachev
Copy link

hdachev commented Feb 10, 2015

Adding an example of broken return value inference that results in a compiler error:

type Events = {
    [event: string]: () => void;
};

class A {
    getEvents(): Events {
        return null;
    }
}

class B extends A {
    getEvents() {
        return {
            'click': () => {

            }
        };
    }
}

// test.ts(12,7): error TS2415: Class 'B' incorrectly extends base class 'A'.
//   Types of property 'getEvents' are incompatible.
//     Type '() => { 'click': () => void; }' is not assignable to type '() => { [event: string]: () => void; }'.
//       Type '{ 'click': () => void; }' is not assignable to type '{ [event: string]: () => void; }'.
//         Index signature is missing in type '{ 'click': () => void; }'.

@omidkrad
Copy link

+1 vote for this. Related issues: #178 and #2073

I think it's not intuitive for contextual typing not to apply when overriding a class member. i.e. it should.

@j-walker23
Copy link

+1

1 similar comment
@Coderah
Copy link

Coderah commented Oct 31, 2015

+1

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. labels Nov 3, 2015
@RyanCavanaugh
Copy link
Member

Approved. When a method has multiple parent declarations (e.g. implementing multiple interfaces which each have a method named the same thing), union them to provide a contextual type.

@sandersn
Copy link
Member

sandersn commented Feb 1, 2016

This will be fixed by #6118.

@mhegazy mhegazy added Committed The team has roadmapped this issue and removed Help Wanted You can do this labels Feb 22, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Feb 22, 2016
@tinganho
Copy link
Contributor

Can the scope of this be expanded to support member properties and static properties? See my comment in #1887 for an example.

I'm also in support for extending it to base class members and statics.

@sandersn sandersn added the Design Limitation Constraints of the existing architecture prevent this from being fixed label May 6, 2016
@sandersn sandersn added the Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it label May 6, 2016
@sandersn
Copy link
Member

sandersn commented May 6, 2016

Unfortunately, we couldn't come up with a solution that was consistent and backward-compatible. See #6118 for details. I'm closing this for now.

@sandersn sandersn closed this as completed May 6, 2016
@mhegazy mhegazy removed the Committed The team has roadmapped this issue label May 6, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed Suggestion An idea for TypeScript Won't Fix The severity and priority of this issue do not warrant the time or complexity needed to fix it
Projects
None yet
Development

Successfully merging a pull request may close this issue.