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

Allow accessors in ambient classes and interfaces #32772

Closed
wants to merge 4 commits into from

Conversation

rbuckton
Copy link
Member

@rbuckton rbuckton commented Aug 9, 2019

As part of the effort to support ECMAScript class fields, we will need to roll out features to the TypeScript compiler in phases. One of the key areas we need to address is the ability to properly distinguish between a "field" (a.k.a. a "Property Declaration" in TypeScript parlance), and an "accessor" (a getter or setter declaration). The reason we need to have these definitions cleanly separated is so that we can eventually issue error messages when you define a field on a superclass and an accessor on a subclass with the same name (or vice versa).

However, this is problematic today for several reasons:

  • Subclasses currently can override a property declaration from a superclass with an accessor on a subclass:
    var Map: MapConstructor;
    interface Map<K, V> {
       readonly size: number;
       ...
    }
    
    class MyMap extends Map {
      get size() {
        return super.size + 1;
      }
      
    }
  • Declaration emit for class declarations converts get/set accessor pairs into property declarations and get-only accessors into readonly property declarations:
    // input: main.ts
    class MyClass {
      get x(): number { return 1; }
      set x(value: number) {  }
    }
    
    // output: main.d.ts
    declare class MyClass {
      x: number;
    }
  • Interfaces can specify property declarations, but can't specify accessors.

This PR seeks to address some of these issues:

  • Ambient Class declarations may now specify getters/setters.
  • Interface Declarations may now specify getters/setters.
  • Code fixes for implementing an interface or an abstract class will now add implementations for getters/setters instead of fields when the implemented member is a getter or setter.
  • Getters and Setters in Interfaces are emitted as Getters and Setters in the output declaration file.
  • Getters and Setters defined on an ambient class are preserved when emitting an output declaration file.

This PR does not change the following behavior:

  • Getters and Setters on a non-ambient class declaration will continue to be emitted as a property declaration.
  • It is still not an error to have a subclass and superclass have a mismatch in a definition of a property that is either an accessor or a property declaration.

We are not changing the declaration emit for getters/setters immediately so that declaration files built against existing TypeScript code prior to this feature can still be used with earlier versions of the TypeScript compiler. We do plan to change the declaration emit in a future release, which will give users a transitional period where they can become accustomed to the new syntax.

Examples:

interface Map<K, V> {
  get size(): number;
  has(key: K): boolean;
  get(key: K): V | undefined;
}

declare class C {
  get x(): number;
}

type T = {
  get y(): number; // should work here too...
};

Related #27644

@rbuckton
Copy link
Member Author

rbuckton commented Aug 9, 2019

@weswigham, @RyanCavanaugh, @sandersn, @ahejlsberg, @DanielRosenwasser for review since GitHub's suggested reviewers aren't working for me right now.

@rbuckton
Copy link
Member Author

rbuckton commented Aug 9, 2019

There is more we need to consider in this space, so we are going with a narrower PR that only targets ambient classes at this time. This has been superseded by #32787 and will be revisited in the future.

@rbuckton rbuckton closed this Aug 9, 2019
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

Successfully merging this pull request may close these issues.

1 participant