Class-fields.md

Plan

In 3.6

1. Allow accessors in d.ts files.

For 3.7

1. Emit accessors in d.ts.
2. Two new errors: one for Define semantics, one for uninitialised properties.
3. Add a flag to emit Define when on, silence the new errors when off.

Changes

Disallow accessor/property overrides

• Properties can only override properties
• Accessors can only override accessors
• Except when the base is abstract or an interface
  • abstract: nothing there to interfere
  • interface: we pretend there's not
• Open question: whether to provide codefixes for either error.

Property overrides accessor

• Property overriding accessor doesn't work with Define.
• Assuming that 'work' means that the base gets to do something clever.

class CleverBase {
  _p: unknown
  get p() {
    return _p
  }
  set p(value) {
    // cache or transform or register or ...
    _p = value
  }
}
class SimpleUser extends CleverBase {
  // base setter runs, caching happens
  p = "just fill in some property values"
}

• SimpleUser is broken when switching to Define semantics.
• So we'll issue an error.
• Migrating is simple but not immediately obvious.

class SimpleUser extends CleverBase {
  constructor() {
    // base setter runs, caching happens
    this.p = "just fill in some property values"
  }
}

Accessor overrides property

• Accessor overriding property with Set almost lets you avoid the base class property.
• But the base class still calls the derived accessor when setting the base property.
• Most people who do this will think they are being clever.
• And ... it doesn't work at all with Define.

class LegacyBase {
  p = 1
}
class SmartDerived extends LegacyBase {
  get() p {
    // clever work on get
  }
  set(value) p {
    // additional work to skip initial set from the base
    // clever work on set
  }
}

• SmartDerived is broken when switching to Define semantics.
• So we'll issue an error.
• Migrating is simple, but not obvious and does not have exactly the same semantics
• Specifically, "additional work to skip initial set" needs to be removed.
• If it was ever there in the first place...

class SmartDerived extends LegacyBase {
  constructor() {
    Object.defineProperty(this, "p", {
      get() {
        // clever work on get
      },
      set(value) {
        // clever work on set
      }
    })
  }
}

Breaks

Scattered all over large codebases. There are not a large number of breaks, but 6 RWC projects had failures. None in user tests. DT has 2

Property overrides accessor

1. Angular 2: the derived class has a decorator which mentions the derived property. I have no idea what the intended semantics are here.

Accessor overrides property:

1. the copies of VS Code and Azure SDK that we have are so old they didn't have abstract properties. Base should be abstract.
2. POPULAR MICROSOFT APP 1a: an accessor pair overrides a property, intending to make it readonly.
3. POPULAR MICROSOFT APP 1b: The getter returns a constant, so seems straightforwardly replaceable with a property. The comment above says "has to be a getter so overriding the base works correctly". Uh-oh.
4. Babylon: Same, except the accessors are all readonly.
5. skatejs: Could have used a property

Disallow uninitialised override properties

• Property declarations without initialisers that override a base property must use new syntax:

• class C extends B { declare p: number }

• This declaration is erased

• Without declare, issue an error.

• A codefix inserts declare where needed.

• Open question: syntax bikeshed

• The error is not issued when:

  • The derived property is already in an ambient context
  • TODO The base is in an interface
  • The base or derived properties are abstract
  • The derived property results from a mixin (there is no actual declaration then)
  • The property is initialised in the constructor
  • (note: without strictNullChecks we can't detect this, so always issue the error)

Examples

class B {
  p: number
}
class C extends B {
  p: number
}
class D extends B {
  p: 1 | 2 | 3
}

• C and D now need to be written as:

class C extends B {
  declare p: number
}
class D extends B {
  declare p: 1 | 2 | 3
}

Breaks

No failures from user baselines. 19 failures from DT. RWC had around 100 failures.

Most of the uses I saw were trying to declare the existence of a property or refine its type. They want exactly this syntax, although they would be unlikely to know it until given an error.

1. Declaring a more specific type:
   1. Deep, entangled hierarchies like Azure SDK, VS Code.
   2. Parametrisation through overriding like ember, React Native, react-router, React.
   3. Both are technically the same; both are usually unsound.
2. Redundant redeclarations, eg Babylon, F12, Firestore, Skype, Polymer. This could be
   1. cut-and-paste
   2. a love of locality.
   3. not being sure whether the base defines the property.
   4. Wishing for abstract properties in old code bases.

The remaining uses were declaring the property and initialising it in the constructor. With strictNullChecks they would not have received the error, but RWC code mostly predates that.

Always emit accessors in d.ts

class C {
  get p1() { return 1 }
  get p2() { return 1 }
  set p2(value) { }
  set p3(value) { }
}

Now produces

declare class C {
  get p1(): number;
  get p2(): number;
  set p2(value: number);
  set p3(value: any);
}

Previously it produced

declare class C {
  readonly p1: number;
  p2: number;
  p3: any;
}

Flag for emitting Object.defineProperty

• When "legacyClassFields": true:
  • Initialized fields in class bodies are emitted as constructor assignments (even when targeting ESNext).
  • Uninitialized fields in class bodies are erased.
  • The preceding errors are silenced.
• When "legacyClassFields": false:
  • Fields in class bodies are emitted as-is for ESNext.
  • Fields in class bodies are emitted as Object.defineProperty assignments in the constructor otherwise.
• In 3.7, "legacyClassFields" defaults to true
• Declaration emit is the same regardless of the flag's value.

Examples

class C {
  p1 = 1
  p2
  [computed] = 2
}

with "legacyClassFields": false, downlevels to:

class C {
  constructor() {
    Object.defineProperty(this, "p1", { value: 1 });
    Object.defineProperty(this, "p2", { value: undefined });
    Object.defineProperty(this, computed, { value: 2 });
  }
}

with "legacyClassFields": true, downlevels to:

class C {
  constructor() {
    this.p1 = 1;
    this[computed] = 2;
  }
}

Note:

1. This transform happens even when the target is ESNext.
2. p2 is erased.