Improving the overall type safety of the entire library #2917
Conversation
|
| "alwaysStrict": true, | ||
| "noUnusedLocals": true, | ||
| "noUnusedParameters": true, | ||
| "skipLibCheck": true, |
There was a problem hiding this comment.
I am not sure if running type check for dependencies is a good idea. This might be overhead for our build process.
There was a problem hiding this comment.
It was an oversight by me. Thank you for spotting it.
| @@ -15,4 +15,5 @@ module.exports = { | |||
| moduleNameMapper: pathsToModuleNameMapper(tsconfig.compilerOptions.paths, { prefix: `${ROOT_DIR}/` }), | |||
| collectCoverage: false, | |||
| cacheDirectory: resolve(ROOT_DIR, `${CI ? '' : 'node_modules/'}.cache/jest`), | |||
| preset: 'ts-jest', | |||
There was a problem hiding this comment.
I avoided to use ts-jest on purpose in order to have faster compilation and reduce overhead of tsc. I don't think we need type checking in tests. At least, it is not CI's job I believe. So it's done by babel already. I thought of removing ts-jest completely but I realized that we still need it for pathsToModuleNameMapper above to use paths from tsconfig file.
There was a problem hiding this comment.
I would agree that for most cases the CI should not have to run type checking. But for open source tools and libraries with advanced generic types like the ResolversComposerMapping type, they should absolutely be tested . They are still a written piece of code and should be treated as such since they are prone to errors like any other code.
Tsc and babel only validate that types are correct. They don't know how to test intended behavior within generic types and ultimately allows code like this to pass type checking
interface Resolvers {
Query?: {
foo: () => string
}
}
export const rcMap: ResolversComposerMapping<Resolvers> = {
Query: {
foo: 'hello'
}
}I understand that faster compile times and less overhead is desired. But if it compromises type robustness then it's not a great trade-off and makes using said types superfluous since they might or might not work as intended.
Description
This PR is an effort to improve the overall type safety of the entire library.
The lack of strictness is causing unintended behavior for projects relying on stricter type safety. To fix these issues typescript strictness rules have to be tightened. Types and code will have to be re written to conform to the stricter type checks.
Related #2907
Type of change
Further comments
This PR will take some time to complete but eventually give much stronger type safety.