Added error for class properties used within their own declaration

[JoshuaKGoldberg]

Fixes #5987.

Usages of a class property in a preceding property already gave an error, but the following doesn't yet:

class Test {
    x: number = this.x;
}

As with other use-before-declare checking, IIFEs are not treated as invalid uses.

[JoshuaKGoldberg]

Ping @sandersn, any chance this could be reviewed please? 😄

[sandersn]

1. Need to flip the boolean return value.
2. Need to merge from master.
3. How is the code in isInPropertyInitializer used?

Otherwise looks like exactly the thing.

[sandersn]

Hm, build still fails on tests/cases/compiler/classUsedBeforeInitializedVariables.ts. Does this need another merge from master?

[chriskrycho]

Hey folks, a quick note here: we've been relying on this behavior as a workaround for using classes in older versions of Ember (<3.6), and I don't think there's (guaranteed to be?) a TDZ here. The current spec doesn't mention one, and in the case of subclassing it's valid:

class Foo {
  baz = "hi"
}

class Bar extends Foo {
  baz = this.baz !== undefined ? this.baz : "hello"
}

let bar = new Bar();
bar.baz; // "hi"

The error correctly doesn't come up when subclassing (as in this example)… but not all instances of subclassing (where this is valid) can be made transparent to TS

Both Chrome's shipped and Firefox's feature-flagged implement the feature that way ☝️, as does the Babel implementation, and Ember's legacy class construction behaved in such a way as to require something like that (or some other workaround) when interoperating ES classes with its legacy classes.

Edit: to elaborate, in Ember versions prior to 3.6, the instance would already have a value set on it in certain cases courtesy of the implicit super() invocation, so it's equivalent to the subclassing example, but in a way that's invisible to TS. (This was a source of considerable pain for us along the way, as you can imagine; this was one of the data points that helped us design a better implementation in 3.6+.)

[sandersn]

@chriskrycho can you give an example of the legacy class + ES class failure? I guess that it's something like

class Bar extends LegacyFooGen() {
  baz = this.baz + 1 // overrides LegacyFooGen().baz, but Typescript doesn't know that.
}

Maybe we can restrict the scope of this error to avoid pre-3.6 Ember.

[chriskrycho]

Yes! I should have done that in the first comment; sorry. The common pattern looks like this:

import Component from '@ember/component';
import defaultTo from 'lodash/default-to';

export default class Welcome extends Component {
  greeting: string = defaultTo(this.greeting, "Hello");
}

(Most folks doing this are using defaultTo or similar to work correctly with truthy types.) This is to support optional component arguments, so users could do either of these:

<Welcome />
<Welcome @greeting="Bonjour" />

Prior to Ember 3.6, the constructor for Component sets up arguments passed in the template invocation. For 3.6 and later, that gets handled differently by the framework (i.e. not in the Component constructor), so the problem goes away; people can just write greeting = "Hello" as the initializer, and the framework takes care of setting any arguments after construction fully completes.

[sandersn]

Just curious, is the fact that some uses of <Welcome /> have @greeting the only static marker that Welcome will get this.greeting initialised by Component ?

[sandersn]

@weswigham and I thought of two fixes, which we would probably want together:

1. Allow this error to be squelched with postfix-!, adding it to the category of property initialisation patterns that are known to be unsafe, but common enough to allow. (There is only one pattern that I know of right now: property!: string;)
2. Only make this an error with "strictPropertyInitialisation": true.

I think this is usually a good error, so I'd prefer it to show up most of the time. However, strictPropertyInitialisation behaves as if every property declaration had a postfix-!, so it would make sense for things-squelched-by-! to also be squelched by that flag being false.

I'll create an issue and we can talk about it at a design meeting.

[chriskrycho]

Yeah, and there's currently no type data emitted from that template context into the TS context. This is on the Ember TypeScript team's radar, but not implemented yet.

(I looked at type workarounds, but unfortunately, because component args are just smooshed into the component directly, none of them work.)

I think that suggested strictPropertyInitialization rule makes sense and would solve this problem. Thanks for the quick response!

[jamescdavis]

Chiming in here (I originally discovered this issue), I think the proposed solutions make sense and I'm glad there will be a way to squelch the error on a case-by-case basis (with !) as I don't want to have to disable strictPropertyInitialization globally. Thanks for considering our special needs!