build: enforce strict checking in tsconfig.json
#242
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit b442e3a.
When useUnknownInCatchVariables is disabled, the thrown error in the catch block is type any. If you use unknown in catch variables, you might be tempted to use `instanceof` in order to narrow it to an Error type. But this check returns false if you threw a class that inherits from Error. Why? This is because we currently target ES5 in the `tsconfig.json` file instead of ES6/ES2015. Keeping it this way is dangerous for the codebase because it is easy to unintentionally introduce a bug in an effort to keep the code type-safe, since TypeScript won't warn us about this. References: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#custom_error_types https://github.com/microsoft/TypeScript/wiki/FAQ#why-doesnt-extending-built-ins-like-error-array-and-map-work
|
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #236
What Changed
strictmode andforceConsistentCasingInFileNames. DisableduseUnknownInCatchVariables.strict: true, mainly in theschemapackage.numberEnum.Problems
Some compromises had to be made to enable this without it taking weeks of time.
unknownInCatchVariablesis set to falseWhen useUnknownInCatchVariables is disabled, the thrown error in the catch block is type any. If you use unknown in catch variables, you might be tempted to use
instanceofin order to narrow it to an Error type. But this check returns false if you threw a class that inherits from Error. Why?This is because we currently target ES5 in the
tsconfig.jsonfile instead of ES6/ES2015.Keeping it this way is dangerous for the codebase because it is easy to unintentionally introduce a bug in an effort to keep the code type-safe, since TypeScript won't warn us about this. We should upgrade to ES6.
References:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error#custom_error_types
https://github.com/microsoft/TypeScript/wiki/FAQ#why-doesnt-extending-built-ins-like-error-array-and-map-work
In some places,
@ts-ignoreand any are usedThis was done in order to make migrating to
strict: trueeasier. The point ofstrict: truewas to enforce strict type checking on all new code in the codebase. The benefit of strict type safety for all future code is worth the risk.In some tests,
@ts-expect-erroris usedThis was intentional. It's a problem with using TypeScript with Jest. Using Jest expect functions does not narrow down the type. Ideally, there'd be assertion functions we could use to assert the type but it's not available yet.