Several issues with strict property checks and definite assignement assertion

[cyrilletuzi]

TypeScript Version: 2.7.0-dev.20171214

This issue is about the new strict property checks #20075 and definite assignement assertion #20166.

@ahejlsberg @mhegazy @DanielRosenwasser @RyanCavanaugh Did you guys test this new feature on real world projects ? Because as it is now, it could lead to a huge (negative ?) impact. It may need more thinking/fixing before landing in stable.

As VS Code Insiders now uses TS 2.7, when opening my Angular project (which is in strict mode), I discovered with surprise that now half my files contain errors (all my components) due to this new feature. And trying to get compliant with this led to many problems, some of them with no solution currently.

Take this very simple Angular code :

import { Component, Input } from '@angular/core';
import { Movie } from './movie';

@Component({
  selector: 'app-movie',
  template: `<div><h1>{{movie.title}}</h1></div>`
})
export class MovieComponent {

  @Input() movie: Movie;

}

movie property now raises an error as TS can't know it will be initialized by an Angular mecanism.

Let's look at the solutions to be compliant with the new strict mode feature :

1. Putting default values everywhere :

@Input() movie: Movie | null = null;

Unfortunately, that's not possible here, as an object is expected. Only solution would be to set null by default, but then strict mode would raise another error due to strictNullChecks. So you would allow null as a type, but then TS will scream everywhere you use movie inside the class and force you to check if it's not null, when you already know very well it's not. So it's a mess.

Another problem : this feature checks the constructor, but in Angular you often must wait another callback like ngOnInit to do your initial stuff.

2. Use the new definite assignement assertion :

@Input() movie!: Movie;

First, in Angular, it would mean quite all the components properties will need this "hack". As in reality, the code is OK, it seems to me it's a too overwhelming solution to just calm down TypeScript. It could lead people to never adopt this feature, and then complicate the usage of the combined strict option, as we would need to just activate some individual strict features. Did you guys ask the Angular team what they think about this ?

Second, it's now Angular which raises an error in the template, saying Identifier 'title' is not defined. '<anonymous>' does not contain such a member. We should verify before landing that the Angular language service is able to fix this issue. @chuckjaz

Third, TS Lint is not happy too : it's saying Missing semicolon (semicolon) just after the !. It also should be fixed before this feature is landing to stable.

[ahejlsberg]

I agree this feature is likely to cause new errors in existing code which is why we've placed in it the --strict family of switches. I also agree it will cause "false positives" on properties that are initialized by some injection mechanism (such as @Input) which is the reason we're providing ! definite assignment assertions. Even so, it is the consensus on the design team that (a) a lot of people really want this feature and (b) it is not a lot of hardship to disable it with a --strictPropertyInitialization false in projects that are using --strict. After all, when you opt into --strict you implicitly acknowledge that new errors may be reported by new versions of the compiler (see rationale in #14486).

First, in Angular, it would mean quite all the components properties will need this "hack".

I wouldn't characterize it as a hack. It is an explicit acknowledgement that the particular property is initialized by some external mechanism that isn't visible to the type checker (or the casual reader of the code). Some would even argue it is good from a documentation perspective.

Second, it's now Angular which raises an error in the template, saying Identifier 'title' is not defined. '' does not contain such a member. We should verify before landing that the Angular language service is able to fix this issue. @chuckjaz

Third, TS Lint is not happy too : it's saying Missing semicolon (semicolon) just after the !. It also should be fixed before this feature is landing to stable.

Agreed on both of these.

[cyrilletuzi]

I think the idea of this feature is good too, I just think the practical solutions may not be good or enough as they are now.

I understand that it's normal in strict mode to have new errors raising. But normally the new errors are about code that was already badly designed before, but undetected. The problem here is that it raises a lot of error for code that is well designed, and so to be compliant it needs a code which is not classis JS in too many places.

Wouldn't it be possible for TypeScript to automatically acknowledge other mecanisms (like decorators) which are in charge to initialize properties, instead of having to manually use ! quite everywhere ?

[kitsonk]

Wouldn't it be possible for TypeScript to automatically acknowledge other mecanisms (like decorators) which are in charge to initialize properties, instead of having to manually use ! quite everywhere ?

#20166 is specifically designed to make this possible where this was out of the visibility of CFA. Currently we have no choice but to use the null assertion operator. Decorators not augmenting types has been there as long as decorators have been supported. I would love to see some sort of ambient decorators and that is still on the long term roadmap, but it is clearly a challenging situation of providing the sort of syntax necessary to describe all the possible things that a decorator could do. There isn't a solid concise proposal for this sort of syntax and I am really glad we have at least something that overrides CFA at level that persists compared what we had before. I would be really surprised that #20166 is causing you errors, it is an escape hatch to deal with other errors.

We (@dojo) always expect a level of refactoring when upgrading versions of TypeScript. We use --strict mode exclusively and the upgrade to 2.6 was rather tough... We were playing loose with some co-variance allowed with functions. While we didn't identify any issues, we had to clean up some stuff to make sure it was sound going forward.

Did you guys test this new feature on real world projects ?

BTW, the core team does test against real world projects.

[chuckjaz]

Second, it's now Angular which raises an error in the template, saying Identifier 'title' is not defined. '' does not contain such a member. We should verify before landing that the Angular language service is able to fix this issue. @chuckjaz

@cyrilletuzi Angular's language service will support this syntax when Angular itself is updated to support 2.7 and the language service is updated with the new version of the compiler. This error is currently produced because the language service is using 2.5 to compile syntax introduced in 2.7. Our current plan is to support 2.6 in 5.2 of Angular and 2.7 in the next minor release of Angular after 2.7 is released.

I also agree with Anders that adding ! is not a hack. It is informing the compiler that some other mechanism for which it is unaware ensures that this property is initialized. This is similar in character to the post-fix ! operator which has the same purpose for expressions.

@ahejlsberg, you might consider allowing libraries to declare a decorator such as @Input() to be to a signal to the type checker that members decorated with the decorator should be treated as initialized. You might also consider allowing a class decorator to specify a constructor helper that would allow, for example @Component(), to declare that ngOnInit should be considered a constructor helper and member initializations in this method be should treated as if they occurred during construction. These two additions should allow us to declare the intent of the framework to the type-checker making adoption of this feature relatively trivial for Angular developers as well as being general enough to capture other patterns such as property-based dependency injection.

Although, not directly helpful for Angular, you might also consider allowing a base class to declare constructor helper methods.

[cyrilletuzi]

We (@dojo) always expect a level of refactoring when upgrading versions of TypeScript. We use --strict mode exclusively and the upgrade to 2.6 was rather tough... We were playing loose with some co-variance allowed with functions. While we didn't identify any issues, we had to clean up some stuff to make sure it was sound going forward.

Again, the problem is not refactoring in itself. Refactoring because of badly designed code is OK. For example, the TS 2.6 strictFunctionTypes indeed generated a lot of errors, because many of us (myself included) had the bad habit to cast the wrong way.

But refactoring (and a huge one) when the code is well designed but TypeScript is not able to detect it, it is not the same thing.

Again, I think too strict property checks is a good idea, but UX should be considered too and the definite assignement assertion solution doesn't seem user-friendly to me in its current state. It will add complexity to JavaScript and TypeScript beginners when it could be TypeScript doing a better job.

If TypeScript is able to manage things like decorators, it would be a far better solution and it would be relevant to not rush.

[cyrilletuzi]

In my opinion, the fact that strict property checks had to immediately be accompanied by the definite assignement assertion shows there is a problem somewhere.

The reasoning behind the ! solution is :

1. It would be nice to check properties initialization (good !)
2. but wait : it will cause problems in many cases
3. so let's the developer change all his/her code (even when it's good code) to manage these problems

This reasoning would be better :

1. It would be nice to check properties initialization (good !)
2. but wait : it will cause problems in many cases
3. so let's TypeScript manage these problems

[ahejlsberg]

@chuckjaz:

you might consider allowing libraries to declare a decorator such as @Input() to be to a signal to the type checker that members decorated with the decorator should be treated as initialized.

Yes, we discussed that option, but until the decorators proposal reaches stage 3 we're unlikely to go there (we already have the headache of reconciling our current decorator support with the final proposal).

You might also consider allowing a class decorator to specify a constructor helper that would allow, for example @Component(), to declare that ngOnInit should be considered a constructor helper and member initializations in this method be should treated as if they occurred during construction.

Right, although it think this it is less obvious how to generalize this one.

@cyrilletuzi: We're definitely committed to improving this feature over time (e.g. by recognizing special decorators), but the design team feels that, in the balance, the feature is valuable enough in its current state to be a --strict switch. Part of the reasoning here is the ease with which the feature can be disabled if it is deemed too intrusive.

[RyanCavanaugh]

Given the vast majority of decorators are Angular ones that initialize a property, should we just assume any decorator on a property is equivalent to ! ?

[kitsonk]

Given the vast majority of decorators are Angular ones that initialize a property, should we just assume any decorator on a property is equivalent to ! ?

I think you could strike Angular and replace it with almost anything. While a decorator could be setting up a property only for some sort of trapping, it will still likely perform some sort of assignment, setup a value, whatever. It feels to me like until decorators are revisited and the potential for CFA to understand them better, seeing a property decorator as an initialisation action feels appropriate.

[chuckjaz]

@ahejlsberg:

Right, although it think this it is less obvious how to generalize this one.

Consider allowing a postfix ! annotation on methods that would indicate that it is an initializer such as:

@Component({...})
export class MyComponent {
  private myData: MyData;

  ngOnInit!() {
    myData = calculateMyData();
  }
}

In this example, the ! after ngOnInit indicates it is a constructor helper and is treated as if it was called as part of object construction.

You can then imagine a class transformer such as,

type AngularComponent<T> = T & { ngOnInit?!(): void; };

function Transformer<T>(clazz: Class<T>): Class<AngularComponent<T>> {
  ...
}

which says something like, if T has a method ngOnInit, treat it as if it was declared as having ! (the syntax above is not exactly right but should be take as illustrative). We could then change the Component decorator to return a function of such a type.

A class decorator would then be interpreted as if it was a class transformer and the type of the class would be the function's return type.

This is general in that it can specify any arbitrary functional composition (or higher order classes) that can be described using a function type.

A similar thing could be done with member annotations using mapped types if the name of the property as a type was supplied as an implied type parameter to the decorator.

[cyrilletuzi]

Another feedback : same problem in native Web Components, where initialization is not done in constructor but in connectedCallback. And here, there is no decorator to detect it.

As it's a standard, TypeScript should check initialization in connectedCallback too.