镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Contextually type class property initializers by extends/implements types #3667

Description

When a property has exactly one type from an extends or implements clause, we should contextually type that property's initializer by the type from the clause. For example:

interface ThingListener {
    handleEvent: (x: MouseEvent) => void;
}

class Foo implements ThingListener {
    handleEvent = x => {
        // No error, but this code is incorrect
        console.log(x.timestamp);
    }
}

Ideally we could take this one step further when widening. The following behavior is also undesirable:

interface HasLength {
    length: number
}

class Foo implements HasLength {
    // length: any, not really what we intended
    length = undefined;
}

var x = new Foo();
x.length = 'wat'; // should not be allowed

e.g. #3666

Activity

  1. NoelAbrahams commented on Jun 29, 2015

    @NoelAbrahams

    Is this not the same as #1373?

  2. RyanCavanaugh commented on Jun 29, 2015

    @RyanCavanaugh
    MemberAuthor

    #1373 is about methods, this is about properties

  3. DanielRosenwasser commented on Jun 29, 2015

    @DanielRosenwasser
    Member

    So is this a broader proposal than #1373? In other words, in the following example, is the arrow function in SortByQueryId contextually typed?

    Additionally, if we declared getvalue in ISortInfo as a property but defined it as a method in the class, would its parameter's type be inferred?

        export interface ISortInfo
        {
            getvalue(x: QuerySummary): number | string;
            order: SortDir;
            ordercalc?: number;
        }
    
        class SortByQueryId implements ISortInfo
        {
            getvalue = x => x.QueryId;
            order = SortDir.Flip;
        }
  4. RyanCavanaugh commented on Jun 29, 2015

    @RyanCavanaugh
    MemberAuthor

    I don't think we should distinguish between method-style and property-style declarations in the interface - both are an equally valid "source" in terms of type information, regardless of how the class is implementing it.

    In the example, I would expect the arrow function to be contextually typed under this proposal, but not #1373.

  5. sophiajt commented on Jul 16, 2015

    @sophiajt
    Contributor

    Agree with Noel Abrahams (@NoelAbrahams) - can we merge this with #1373? If this subsumes that proposal, can we dupe that one to this one?

  6. tinganho commented on Jul 16, 2015

    @tinganho
    Contributor

    Also merge with #3804 ? Then we include interfaces too.

  7. Gaelan commented on Sep 21, 2015

    @Gaelan

    👍

  8. omidkrad commented on Oct 8, 2015

    @omidkrad

    From #5181 the following sample reproduces this issue:

    declare module Backbone {
        interface RoutesHash {
            [routePattern: string]: string | {(...urlParts: string[]): void};
        }
        class Events { }
        class Router extends Events {
            routes: RoutesHash;
        }
    }
    
    declare module Marionette {
        class AppRouter extends Backbone.Router {
        }
    }
    
    module Backbone.Tests {
        // Error, routes is not contextually typed
        class MyRouter extends Backbone.Router {
            routes = {
                "some/route": "someMethod",
                "some/otherRoute": () => {}
            };
        }
    }
    
    module Marionette.Tests {
        // Error, routes is not contextually typed
        class MyRouter extends Marionette.AppRouter {
            routes = {
                "some/route": "someMethod",
                "some/otherRoute": () => {}
            };
        }
    }
  9. RyanCavanaugh commented on Nov 3, 2015

    @RyanCavanaugh
    MemberAuthor

    Tentatively approved -- this would be a slight breaking change, but probably very much in the "good" direction (i.e. uncovers bugs that were going missed before). We will investigate breakages in our RWC suite to see what the net impact is.

  10. omidkrad commented on Nov 6, 2015

    @omidkrad

    I'd be fine with this breaking change.

  11. sandersn commented on May 6, 2016

    @sandersn
    Member

    Unfortunately, we couldn't come up with a solution that was both consistent and backward-compatible. The breaks in our Real World Code suite were more bad than good. See #6118 for details. I'm closing this for now.

  12. added
    Design LimitationConstraints of the existing architecture prevent this from being fixed
    Won't FixThe severity and priority of this issue do not warrant the time or complexity needed to fix it
    on May 6, 2016
  13. locked and limited conversation to collaborators on Jun 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Labels

CommittedThe team has roadmapped this issueDesign LimitationConstraints of the existing architecture prevent this from being fixedSuggestionAn idea for TypeScriptWon't FixThe severity and priority of this issue do not warrant the time or complexity needed to fix it

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions