feat(@schematics/angular): add strict compiler option to workspace tsconfig

[alan-agius4]

Add "stricter" compiler option to workspace tsconfig

Closes #14694

[vikerman]

This needs more discussion. I want to understand the impact and also why the app specific tsconfig has no strictness flags set.

[clydin]

From a review of similar tools, the following all set strict by default:

• create-react-app
• vue-cli
• firebase-tools

This change would also only affect newly created projects.

[xiaoxiangmoe]

• the demo codes in angular.io need to be updated for strict type check.
• the app specific tsconfig has no strictness flags set because it extends the workspace tsconfig which has strictness flags set.

[filipesilva]

We need to check with our AIO examples if they even work with these options. Some of them are also problematic on their own, like strictPropertyInitialization that doesn't work well with @Input and such. So we should consider them individually.

[xiaoxiangmoe]

strictPropertyInitialization works well with definite assignment assertion modifiers. see Strict Class Initialization

class C {
    @Input() readonly foo!: number;
    //                   ^
    // Notice this '!' modifier.
    // This is the "definite assignment assertion"
}

If you need more help about strict type check, I am happy to help.

---

By the way, I think we should use more readonly modifier in AIO.

[vikerman]

Marking this as requiring change just to move it off the Approved reviews list.

[xiaoxiangmoe]

What do I need to do to get this PR merged?

[vikerman]

@xiaoxiangmoe - On our side we need to turn one flag at a time in our documentation projects to make sure all our documentation works with each of these flags. And then enable it in the CLI by default.

For example adding a "!" is not acceptable without changing our documentation to reflect something like that(and I don't know whether it's a good idea to make everyone do that)

[alan-agius4]

I just had a look at this, and it seems that AIO is already in strict mode with the exception of strictPropertyInitialization which is false.

See: https://github.com/angular/angular/blob/1d3e22766a978bc8bd6e3ef29cec5cdb44e04278/aio/tsconfig.json#L22-L25

I have updated this PR to reflect the AIO settings.

Outdated review

[xiaoxiangmoe]

@alan-agius4 can we enable strictPropertyInitialization in angular 9?

[alan-agius4]

@xiaoxiangmoe, while I cannot say for certain. I highly doubt that we would be able to enable strictPropertyInitialization in version 9. Especially due to the @Input issue explained above.

stale review

[xiaoxiangmoe]

@alan-agius4 Maybe we can provide some instructions in the documentation -- what do we need to do if we want to enable strictPropertyInitialization.

[alan-agius4]

Hi @xiaoxiangmoe, fyi there is an issue in angular/angular related to strictPropertyInitialization angular/angular#24571. That said, at this point in time I would not recommend using strictPropertyInitialization.

[vikerman]

The feedback from Angular team on latest proposal -

1. Let's ship this for a major release(9.0) so that it gives us time to communicate to people who write tutorials
2. Have an option to turn off all strict flags in case someone wants to quickly prototype something (and something like noImplicitAny might come in the way)

[imcotton]

This @Input does not guarantees its instance to get a non-null / non-undefined property (e.g. [foo]="obs$ | async"), therefore Component author should always anticipate its property as optional (e.g. @Input() foo?: number), which a) gains type safety b) complies with the strict restriction.

End users should fully embrace the strict checking from TypeScript, set strictPropertyInitialization to false here only because @Input by CLI by default is like to choosing a sad path for user before they were taking steps, it's not a defect, not necessary to compromise at the first place.

[alan-agius4]

Blocked until we get a list of which options we should to enable by default.

@vikerman will checks the order of option that were enabling in G3 and we’ll decide based on that.

[vikerman]

The current list which we think is basic for most samples/examples in the wild and also our ecosystem is

    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noFallthroughCasesInSwitch": true,
    "strictNullChecks": true

[Splaktar]

I really like the idea of having a strict option and the ability to have it include these compiler flags in tsconfig.json. However, making it the default is a bit concerning. I like the fact that making this the default is only being considered for a major version.

I would really prefer to make this strict option default to false, then use the Analytics to determine if the usage is sufficient to warrant making it the default. This would also give us time to see what kinds of issues people run into with strict mode before making it a default.

Perhaps strict mode that defaults to false is something that could go into a version 8 minor for a month or two? If no more version 8 minors are planned, then perhaps it can go into version 9 with default false and then be considered for enabling the default in version 10?

[Splaktar]

I'd also like to have these settings tested against projects generated using all of the Angular Material Schematics and the material.angular.io docs project (and yes, I recognize that this change would only directly impact newly created projects).

[vikerman]

We should probably set this to false and get the PR in. And move the decision on whether we want to turn in on for 9.0 based on data we collect for this, later.

[clydin]

I think "noImplicitAny": true, should be there regardless of the setting. Without it, a developer is essentially bypassing the overwhelming majority of the benefits of TypeScript and may not actually realize it.

[lacolaco]

I guess just adding a new flag is not enough to get noticed by developers. It may be better to make a prompt like "Would you like to enable TypeScript strict type checking? (y/N)". And I agree on the false by default.

[alan-agius4]

I think for sure we should

1. Turn off by default, add prompt so that it's more discoverable by users
2. We revisit this based on the feedback we get right before version 9 RC.

[vikerman]

I'd also like to have these settings tested against projects generated using all of the Angular Material Schematics and the material.angular.io docs project (and yes, I recognize that this change would only directly impact newly created projects).

The actual Angular material samples tsconfig is here and already using here - https://github.com/angular/components/blob/cf70c4342e7ea277011cf289e721383b7784a18a/src/material-examples/tsconfig-build.json

We will look into what tests we have around testing Material schematics. Thanks.

[Splaktar]

Glad to see the examples using that.

I enabled these checks in material.angular.io's tsconfig and I'm seeing about 30 errors. Half of them are due to noImplicitAny.

It's not the end of the world and I can try to put together a PR to fix this, but it's an indication that people will certainly hit this. I guess the hope is that it will encourage them to write cleaner code, but I think that there will certainly be issues with tutorials, guides, blog posts, and other material that "won't work" in the default Angular CLI after strict becomes enabled by default.

[vikerman]

This is only for new projects. So we do fully expect existing projects to break when users manually switch the strict flags. We need to find a way to get all course creators to ensure that if they create a project created from scratch their code works with it.

[clydin]

If there's a prompt now, I think it would be clearer to just use the actual TS strict option. I think it will be confusing that stricter type checking is not actually strict as is the case for other frameworks.

[Splaktar]

LGTM

[xiaoxiangmoe]

If there's a prompt now, I think it would be clearer to just use the actual TS strict option. I think it will be confusing that stricter type checking is not actually strict as is the case for other frameworks.

Maybe we can consider providing --strict=false --strict=stricter , and then keep the default is --strict=true

[clydin]

The current plan is to slowly move to a more strict configuration over the next set of versions. This is to ensure that both first and third party documentation, training material, courseware, etc. are adequately prepared for the changes.

[Splaktar]

Is there an associated GitHub or JIRA issue for verifying that all examples on AIO compile with the stricter flags? For verifying that CLI, PWA, and Universal Schematics generate code that compiles with the new flags?

I've opened angular/components#17135 to track this for the Angular Material Schematics.

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}