feat(@schematics/angular): strict TypeScript

[cyrilletuzi]

Enable currently TS strict features supported by Angular.

Why this PR ?

Lately, I gave several advanced Angular courses, with developers working with Angular for more than one year. Each time, they were not aware of the strict features of TypeScript and their projects were in non-strict mode.

TypeScript is not in strict mode by default for compatibility reasons, and to be transparent with standard JavaScript. But strict is the recommanded way to do.

One of the main reasons frameworks like Angular choose TypeScript is for error management, because in an app, you can't do quick JavaScript anymore, with no error management, like we were doing during the jQuery era. A single error can break an app.

Then, using TypeScript in Angular in non strict mode is quite a non sense : several parts of your app won't be checked correctly, and thus may contain errors, wrecking all the efforts to code well in the other parts which are checked correctly.

So strict mode should be the default.

But another problem is strict mode config is becoming difficult : we can't just enable strict like in #218, because Angular needs time to update to new TS versions. So currently developers have to enable strict features one by one : there are already 4 in TS 2.5, there will be 5 in TS 2.6 and 6 in TS 2.7.

So it would be really better the CLI enables them by default.

Consequences for current projects

None. This schematic is only applied for new apps, so current apps won't break.

@hansl @filipesilva @Brocco

[devoto13]

I generally support the movement to more strict TS.

Just want to share some feedback. I was recently enabling strict null checks in ~10K LOC codebase. I had to do decent amount of changes to the completely correct code just to satisfy strict null checks. Often these changes were making code more verbose without any additional benefit. I'm afraid that having them enabled by default will increase friction for less advanced users.

I'm +1 on enabling noImplicitAny and noImplicitThis by default as they mostly catch real bugs. But I'm not so sure about strictNullChecks. It produces false positives way too often. I'm not yet convinced that real-life benefit worths the added burden.

[cyrilletuzi]

Hum, strictNullChecks is not supposed to produce a lot of false positives. If a data can really be null, invoking a method on it will really produce an error (just an example). Could you add details of common cases where it was a problem ?

[devoto13]

Well, technically they are not false positives :) But they complicate life for a developer.

They mostly come from situations where null assertion is done indirectly. Below are couple of examples from the application:

1. There is a service, which contains a Map instance with some data. When I attempt to get something from the map it can technically be null, but I verified that everything I will ever attempt to get from it actually exists there when I was populating the Map. But TS obviously have no idea about that. So I had to add bunch of wrapper methods with ! assertions to avoid adding checks in every place I try to get item from the Map.

2. Type definitions for D3 library had a lot of inconsistencies with mixing null/undefined, so now I have to use helpers in couple of places to workaround it (didn't get time to submit PRs to fix it yet):

export function undefinedToNull<T>(value: T | undefined): T | null {
  if (value === undefined) {
    return null;
  }
  return value;
}

export function nullToUndefined<T>(value: T | null): T | undefined {
  if (value === null) {
    return undefined;
  }
  return value;
}

3. There is a data, which is guaranteed to be initialized for some components, but not others (that's why type includes null). And actual check is done with ngIf in the wrapper component, but TS obviously have no idea about it. E.g.:

class MyData {
    value: number | null;
}

// in MyWrapperComponent template

<my-component *ngIf="data.value != null" [data]="data"></my-component>

When I attempt to use data.value inside MyComponent it is definitely not null. But I still have to add not null assertions.

[cyrilletuzi]

We can't make a decision with point 2 as an argument : it's a third party lib, and as you say, PR should be made as TypeScript is not guilty here. By the way, you can use skipLibCheck.

Point 3 should be solved now since 5.2 with angular/angular#20702 (trying to test it now).

[devoto13]

Point 2 is an example of how using library not ready for strictNullChecks can be a pain. I'm pretty sure there many out there. skipLibCheck won't help here as .d.ts files are correct. But using methods together in my own code leads to issues, because they are not compatible. Honestly I would be more happy with skipLibCheck: false and strictNullChecks: false than both of them set to true.

RE Point 3: angular/angular#20702 will definitely help with usage inside component template, but developer will still have to add assertions when using object in TS part of the component.

[cyrilletuzi]

strictNullChecks is here since TS 2.0. We're at TS 2.7 now. Can't wait for every lib in the world, we're talking about Angular here, which is compatible with strictNullChecks for a long time. Keep in mind this PR just propose a new good default for generated apps, you'll still be able to disable it in your tsconfig.json if you want.

[devoto13]

I just represent opposite point of view, so we do a weighted decision here ;)

As I said in my case I had to change decent amount of correct code to be compatible and enabling option discovered single place where code was using null instead of '' (empty string). So for me the benefit was quite small comparing to the burden of changing correct code just to make TS happy. Maybe it is completely opposite in other code bases and people will discover a lot of bugs because of enabling the option. Would be interesting to hear such examples.

PS I would vote for enabling it just because I like new things :) And I was very excited about it when I decided to enable it in the project, but it turned out to be not so useful.

[filipesilva]

While I personally like the strict options to make (prod) builds, I don't like them much for development proper. I feel they make it very hard to iterate quickly. That comes down to having different dev/prod configs though.

Separately, I also feel they are extremely hostile to new users. One shouldn't need to code super defensively and understand type theory in order to make a simple Angular app.

If these two points were to be taken together (prod only strict, hard to understand strict errors), then you have hard to understand prod only errors. This is already a big problem with AOT compilation on prod.

On these style issues in general I default to the new user experience. @cyrilletuzi you mentioned specifically your advanced Angular courses, How do you feel these options would affect the beginner courses?

[cyrilletuzi]

I also give my beginner Angular courses in strict mode, and it has never been a problem. On the contrary, TypeScript is very well welcomed, as people very quickly understand that the little additional work to do is very quickly overwhelmed by the great dev experience it enables (I say little work because options like noImplicitAny doesn't imply a lot of extra work, it's just where TypeScript can't infer, and today TS is very clever : as an example, in the 300 lines of ES6 basics I give to my students, there are just 2 lines where types need to be added).

Difficulty is really not on TypeScript during courses, it's often on RxJS.

You take AOT as an example : but it is planned to be the default in dev (if not for performance issues, it would already be the case). fullTemplateTypeCheck will also be the default in Angular 6. So we are clearly on the way to report more and more errors (which is a good thing !). So it would seem more consistent to me to totally assume this direction in regards to TS too.

Of course, my feedback is about students who are oriented by my course, so they understand TS and strict mode are a good thing. People learning by themselves may think wrongly it's too much work for nothing.

But I'll seize the opportunity to raise a major problem with Angular : how it is presented. Because of past communication errors (keeping the name, pushing TS without enough explanations about why it was an excellent choice, lack of communication about semantic versioning, etc.), Angular has acquired the long-term reputation to be difficult. It may lead Vue.js to overcome Angular in 2018, given the last trends.

I think it's a terrible waste, as Angular is of course a very good framework. The documentation should not just explain the technical things. As Angular took some courageous choices (like requiring TypeScript and RxJS), documentation should also educate people about why these choices are good. We're too much in a close world of experts when it comes to Angular. Bridges should be built for beginners.

I would be very pleased to do that. I already wrote a Medium post titled "Angular is easy" about this subject. Let me know if you are open to it.

[filipesilva]

@hansl WDYT? @robwormald/ @StephenFluin your opinion as DevRel would also be important.

[hansl]

Hi @cyrilletuzi

I don't agree with this PR (we should be compatible with those flags but not force them to users). It's relatively simple to add it to your projects, no need to make the default.

I think this should be a flag on new workspaces (something like --strict). If you update your PR I'll let it in. Thanks.

[cyrilletuzi]

@hansl you mean a new option when generating a project with ng new ? If so, maybe ts-strict to avoid collision ?

[hansl]

Sure, that works. However, we don't have a strict option yet, and maybe other options like lint rules could be enhanced as well, so ts-strict is too narrow.

[cyrilletuzi]

Closing in favor of #462