Enforce stricter types via lint when in strict mode

[cyrilletuzi]

🚀 Feature request

Command

• generate library

Description

By default, it is not required to explicitly type functions' return value, as TypeScript can always infer the type based on the function code.

But the inference assumes that the function's code is correct. If the code change or is incorrect, the return type inference will reflect that.

While it is acceptable in applications, as it will raise an error where the function is used if the types do not match anymore, it is more dangerous in a library, as the functions are not used directly. It can lead to unwanted breaking changes in the public API.

Describe the solution you'd like

TSLint has a rule to enforce functions' return type (and ESLint too, for the future migration):

{
  "rules": {
    "typedef": [true, "call-signature"]
  }
}

It could be added when doing ng g library.

It is an easy change as:

• it won't impact applications: libraries already have a specific tslint.json due to --prefix option
• it won't impact already created libraries as it only happens when generating a new one

This is a really easy thing to add, I can take care of the PR if Angular team signal is green.

If it is considered a too big step, it could be added only if the workspace is already using TypeScript strict mode (following the new --strict option in Angular >= 9). But I think as we are talking about libraries, it should really be the default: libraries authors must be stricter to avoid compatibility issues (strict mode should even be the default for libraries, but it would be another discussion).

[alan-agius4]

I personally think that it’s a good pattern to always add a return type even when it’s inferred. Thus, I am in for this changed even for applications.

Let’s see what other team members think about this.

[cyrilletuzi]

@alan-agius4 Thanks for your answer. If you want to have a bigger discussion about strictness, I think the no-any rule should be added too when in --strict: indeed, TypeScript strict is not that strict as it disallows implicit any but not explicit any. So (telling from experience) there can still be a lot of type unsafe code.

It is also great to:

• enforce TypeScript learning (developers often comes to explicit any when their knowledge of TypeScript is too low, eg. not knowing unknown type or generics)
• better conception: when typing becomes too complicated, it often means there is a problem in code conception (it was not always the case, but today TypeScript is able to express nearly all cases).

Note that I know that, due to history, Angular code itself is not compliant with this rule, but it does not impact applications (I use no-any in all my projects with no problem).

[alan-agius4]

Side note: I am not a fan of having have different lint rules in the same mono repo. I think they should be consistent which offers a better DX.

[alan-agius4]

Hi @cyrilletuzi,

We discussed this during our last team meeting and we'd like to do the following:

1. Add "typedef": [true, "call-signature"]
2. In strict mode, add "no-any": true.

For consistence the changes need to be applied in the root level tslint configuration.
https://github.com/angular/angular-cli/blob/master/packages/schematics/angular/workspace/files/tslint.json.template

[cyrilletuzi]

Hi @alan-agius4, PR done: #17834

I'm very happy of this move. I wasn't expecting you would agree to go that way, I think I was the first to propose to add a --strict option some years ago now, did several PR, and they were never accepted.

It's one of the common pitfall I see in Angular projects I audit. Even in small/medium projects, there is quickly hundreds or even thousands of places where the code is not type safe. Leading to the whole projects to be very unreliable, with bugs multiplying over time, refactorings being very risky, etc. And it is very difficult to come back from that (I've passed weeks on some projects to make them type safe).

Next step would be that --strict prompt on ng new would be true by default, but I suppose you don't want to scare beginners.

[alan-agius4]

@cyrilletuzi, the end goal is indeed to make strict mode the default in the future, but we cannot yet. Because we want to make sure that all the courses and docs out there are complaint with strict mode, as don't want to invalid courses, nor make it harder for new developers to use Angular just because some explains are not strict complaint.

I agree that when not using TypeScript in strict mode, you will use only a very small fraction of the potential of the TS type-checker which will lead to bugs which could be caught at an earlier stage.

Thanks for your PR bdw 😁

[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.}