TypeScript definite assignment emit

[matt-tingen]

Bug Report

Current Behavior
In loose mode, @babel/plugin-proposal-class-properties emits a property assignment for TypeScript definite assignments.

Input Code
REPL (I'm not sure if it's possible to turn turn on loose mode for the plugin)
TS Playground

// Like Object.assign but will not assign properties which are already defined
declare const assignDefaults: typeof Object.assign

class Foo {
  bar!: number;

  constructor(overrides: { bar: number }) {
    assignDefaults(this, overrides);
  }
}

Expected behavior/code
A definite assignment should not emit any JS.

// Like Object.assign but will not assign properties which are already defined
class Foo {
  constructor(overrides) {
    assignDefaults(this, overrides);
  }

}

Babel Configuration (.babelrc, package.json, cli command)

{
  "presets": ["@babel/preset-typescript"],
  "plugins": [["@babel/plugin-proposal-class-properties", { "loose": true }]]
}

Environment

• Babel version(s): 7.5.5
• Node/npm version: Node 12.6 / yarn 1.15.2
• OS: macOS 10.14.5
• Monorepo: no
• How you are using Babel: cli

Additional context/Screenshots
This was previously raised in #7997, but considered expected behavior. It was mentioned that it could be reconsidered for loose mode, however.

I do feel like it should be reconsidered one way or another; emitting for these declarations is problematic in the same way emitting for a declare would be. Additionally, this syntax is not valid JS so there the only applicable spec compliance is that of TS.

[babel-bot]

Hey @matt-tingen! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite.

[matt-tingen]

If anyone else runs into this, a workaround is to use declaration merging:

// Like Object.assign but will not assign properties which are already defined
declare const assignDefaults: typeof Object.assign

interface FooOverrides {
  bar: number;
}

interface Foo extends FooOverrides {}

class Foo {
  constructor(overrides: FooOverrides) {
    assignDefaults(this, overrides);
  }
}

[JLHwung]

The TypeScript properties conflicts with tc39 Class Properties proposal. As Babel aligns with ECMAScript, so we would not fix it. In my opinion it should be fixed on TypeScript's side.

The workaround mentioned above is reasonable: simply declare fields in interface.

As this issue duplicates #7644 and it is not a bug that we will fix, I am gonna close it now.

[matt-tingen]

@JLHwung This is not a duplicate of #7644. That issue is asking about non-definite properties without an initializer e.g. class Foo { bar: unknown }. TypeScript does deviate from the proposal in that case, although it's basically a non-issue with strictPropertyInitialization.

In TypeScript, adding the ! makes it exclusively a type annotation similar to declare const foo: number. It's just an assertion to the compiler that something is assumed to happen in the constructor.

Skimming over the class properties spec, I don't see anything which makes class Foo { bar! } syntactically valid JavaScript.

For those reasons, I don't believe that this request conflicts with the proposal. However, if spec compliance is still a concern despite the ! difference, perhaps this could be implemented in loose mode as discussed in #7997.

[JLHwung]

We are not trying to match TypeScript behaviors, the TypeScript parser plugin let you parse TypeScript and apply transformations. Actually behind the scene we model property members in TypeScript as a Class Property in ECMAScript.

It is a valid feature request for loose mode, where we expect less code size. I suggest we also implement #7997 together so that in loose mode a TypeScript property member should be stripped unless it has an initializer.

[nicolo-ribaudo]

I don't think that we need loose mode; we should just strip them in the TypeScript plugin.

[nicolo-ribaudo]

@matt-tingen This bug will be fixed in the next minor release by #10546. Unfortunately we can only fix it when the TypeScript plugin runs before the class properties plugin, since we must remove the field in the TS plugin.
Babel runs plugins before presets, so you will need to either use a preset which includes plugin-proposal-class-properties, or use @babel/plugin-transform-typescript instead of the preset.

[jamescdavis]

I've opened #11129 because definite properties that are decorated should not be stripped. E.g., in Ember, we use a decorator for service injection, which initializes the property:

export default class MyComponent extends Component {
  @service myService!: MyService;
}

If we are using plugins in the correct order, myService is stripped here by the TS plugin before the decorator plugin has a chance to decorate (and initialize) it.

[guilhermesimoes]

Adding a little more detail. This TS code:

class Parent {
  public name: string | undefined;

  constructor() {
    this.name = 'John';
  }
}

class Child extends Parent {
  public name: string;
}

is emitting the following JS code:

var Parent = function Parent() {
  _babel_runtime_helpers_classCallCheck__WEBPACK_IMPORTED_MODULE_9___default()(this, Parent);

  // 4 unnecessary method calls
  _babel_runtime_helpers_defineProperty__WEBPACK_IMPORTED_MODULE_10___default()(this, "name", void 0);

  this.name = 'John';
};

var Child = /*#__PURE__*/function (_Parent) {
  _babel_runtime_helpers_inherits__WEBPACK_IMPORTED_MODULE_6___default()(Child, _Parent);

  var _super = _createSuper(Child);

  function Child() {
    var _this;

    _babel_runtime_helpers_classCallCheck__WEBPACK_IMPORTED_MODULE_9___default()(this, Child);

    for (var _len = arguments.length, args = new Array(_len), _key = 0; _key < _len; _key++) {
      args[_key] = arguments[_key];
    }

    _this = _super.call.apply(_super, [this].concat(args));

    // bug! Child name is now void instead of `John`
    _babel_runtime_helpers_defineProperty__WEBPACK_IMPORTED_MODULE_10___default()(_babel_runtime_helpers_assertThisInitialized__WEBPACK_IMPORTED_MODULE_5___default()(_this), "name", void 0);

    return _this;
  }

  return Child;
}(Parent);

As you can see, Child's name is void instead of John. I'm fixing this by using declare:

class Child extends Parent {
  public declare name: string;
}

But this shouldn't be necessary.

[nicolo-ribaudo]

This is necessary to conform to the ECMAScript spec, TS will probably align to our behaviour in the future.

Ref: microsoft/TypeScript#34787

[guilhermesimoes]

Ah, thanks for the tip, what a rabbit hole. Sorry, but it's a bit hard to follow all the comments, concepts and Babel / TS options.

Is there an option I can pass to Babel to avoid the property being initialised as void when it's already initialised as a class property? Or should I initialise properties in the constructor instead?

And is there an option I can pass to Babel so that declare is the default, and I don't have to add it to uninitialised, typed properties?

[nicolo-ribaudo]

Is there an option I can pass to Babel to avoid the property being initialised as void when it's already initialised as a class property?

No, because it's a behavior observably different from the class fields proposal/specification.

And is there an option I can pass to Babel so that declare is the default, and I don't have to add it to uninitialised, typed properties?

This should already be the default behavior is you use the TS plugin (rather than the preset) and put it before the class properties plugin, so that runs first.

However, I suggest not relying on it (and explicitly use declare) because it will break in the future.

[guilhermesimoes]

No, because it's a behavior observably different from the class fields proposal/specification.

Interesting. What's the rationale for that?

However, I suggest not relying on it (and explicitly use declare) because it will break in the future.

Thanks! Will do so.

[nicolo-ribaudo]

What's the rationale for that?

For initializing properties to undefined? To make sure that the class instances have the same properties as declared in the class declaration.

class X {
  foo;
  
  constructor() {
    this.foo = 2;
  }
}

is always guaranteed to have a foo property, but this isn't the case if foo isn't initialized.

[nicolo-ribaudo]

I noticed that TypeScript emits the Object.defineProperty call even for definite properties:
https://www.typescriptlang.org/play?useDefineForClassFields=true#code/PTAEBkEsGsFNQPICMBWsDGAXAdAQwM76QDmAdqEgK6agDukANg6KQPY0FFmgAOATqx6w+mSLHx0AFpHSTQuPvFwNFuACYBPUGtgAzSKVhqAsACgd6BgvjpWpfB0IlSAET25KDTPgBcoTBpCrLqIqBg4nM5mZpacoABirKygAN5moBQKAIR+pJQAtkjCANzRphm29ph8lFisfAAUrABuwnyQOr6pmXy5BUV8oAC+AJSp6RnyTmRuuh5e+A2Y0vgANKAtbR3iI6Xlw2ZDQA

If you want it to be a type-only annotation, you can use declare instead of a definite property.