Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

[RFC] Typescript type checking options #184

Closed
miroslavstastny opened this issue Sep 5, 2018 · 4 comments
Closed

[RFC] Typescript type checking options #184

miroslavstastny opened this issue Sep 5, 2018 · 4 comments
Labels
RFC vsts Paired with ticket in vsts

Comments

@miroslavstastny
Copy link
Member

miroslavstastny commented Sep 5, 2018

Stardust uses Typescript, but all it's type checking options are disabled, which kills a lot of Typescript's power.
Enabling (some of) the checks would make the code better and it'd easier to enable them in the early stage of the project.

I went through the list of compiler options which could be worth considering and also checked tsconfig in other projects which use Typescript.

option project A project B ionic/react vscode/src rxjs description
noImplicitReturns   true true true true Report error when not all code paths in function return a value.
noImplicitAny (1)   true true   true Raise error on expressions and declarations with an implied any type.
noImplicitThis (1)       true true Raise error on this expressions with an implied any type.
noUnusedParameters   true true     Report errors on unused parameters.
noUnusedLocals   true true true   Report errors on unused locals.
alwaysStrict (1) true     true   Parse in strict mode and emit "use strict" for each source file
strictNullChecks (1)           In strict null checking mode, the null and undefined values are not in the domain of every type and are only assignable to themselves and any (the one exception being that undefined is also assignable to void).
strictFunctionTypes (1)         true Disable bivariant parameter checking for function types.
strictPropertyInitialization (1)           Ensure non-undefined class properties are initialized in the constructor. This option requires --strictNullChecks be enabled in order to take effect.

(1) part of --strict
Default value for all options is false.

Can we think of what would make sense to be enabled?

@kuzhelov
Copy link
Contributor

kuzhelov commented Sep 5, 2018

There was a TS compilation problem caused by --strict flag which has enabled all these checks for our project (#158) - as a result, we've had to (temporarily) disable it.

Agree that proposed strategy to incrementally introduce this functionality back (by means of these discrete flags) is a good option for us, so that we will be able to carefully weight benefits and potential TS compilation problems (increased compilation time, overall stability of compilation process) with each option being introduced 👍

@kuzhelov kuzhelov added the RFC label Sep 5, 2018
@levithomason
Copy link
Member

Note, we resolved to disable strictNullChecks for now and align closely to projects A and B.

@miroslavstastny
Copy link
Member Author

@kuzhelov - AFAIK the problem in #158 was caused by strictNullChecks, so, hopefully the others should be safe for build performance.
stringNullChecks is disabled in other projects as well, we are not going to enable it in stardust for now.

Plan: noImplicitReturns is now enabled by #185, I am going to enable others (noImplicitAny, noUnusedParameters, may be noUnusedLocals and noImplicitThis) - we agreed to enable one more each week and monitor it's impact on the build.

@jurokapsiar
Copy link
Contributor

not valid anymore

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
RFC vsts Paired with ticket in vsts
Projects
None yet
Development

No branches or pull requests

5 participants