medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
469 stars 217 forks source link

chore(#8437): enable useUnknownInCatchVariables rule in tsconfig #9646

Open jkuester opened 1 week ago

jkuester commented 1 week ago

Description

https://github.com/medic/cht-core/issues/8437

The useUnknownInCatchVariables rule basically just makes it so that the TS compiler considers errors in a catch block to have the unknown type (instead of the any type). This necessitates proper type checking when referencing properties on the error. In general I think the "correct" way to handle unknown types is to establish a formal expected type for the value and then create a type guard (or type assertion) to make sure we have the data we expect. However, for catch blocks in particular this is tricky since really the error could be anything, coming from anywhere. For the most part our catch logic just want to grab a (possibly undefined) property value off of of the error and then move on. It does not seem super useful to make the error a formal data type. With that in mind, my approach here was just to create a couple utility functions to make it easier to read properties from an unknown in a type-safe way. Nothing fancy, but it works with minimal additional code needed in the catch blocks. However, I am far from being convinced this is the best approach, so I am very open to other suggestions for better ways to handle this.

Since, we do not have any formal schema validation library, I had to roll my own logic here for checking the properties. I had trouble figuring out where in the webapp code base I should put these functions. All the other code seems to be organized/categorized by specific Angular features with no where that I could just add some general-purpose utility functions. So, I ended up creating webapp/src/ts/libs and adding the schema.ts file to hold my functions. Once again, very open to alternate suggestions here...

Then I had the issue of how to test the webapp/src/ts/libs. It does not need karma tests since it is not Angular-specific. Instead I just wanted plan mocha tests in TS files. Thankfully, in a separate branch I had already been working on adding support for this. So, I pulled that code in here. In general, this is all very excessive just to get these two little schema functions. However, my thought is that this is not so much a one-off as it is a new core pattern that can be leveraged by more code in the future. (As noted, I already have plans to add additional tests like this in a separate branch....) Main thing is that it is providing the opportunity for adding code/tests that is not tightly coupled to Angular.

Code review checklist

Compose URLs

If Build CI hasn't passed, these may 404:

License

The software is provided under AGPL-3.0. Contributions to this project are accepted under the same license.