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
467 stars 217 forks source link

Enable `strict` mode for TypeScript Config #8437

Open jkuester opened 1 year ago

jkuester commented 1 year ago

Describe the issue Typescript offers a broad range of functionality/checks that it can do to ensure program correctness. Many of this are only enabled if the strict flag is enabled. Currently we do not have this flag set and are not taking advantage of all the of the guarantees that TS could provide.

Describe the improvement you'd like Enabling strict all at once would likely require a significant amount of code changes to our existing code base to be compatible with the strict requirements. Instead, we can roll out changes for individual sub-rules one-by-one and slowly work our way towards being able to just enable strict (and thereby automatically take advantage of future checks added to Typescript).

Each of these will need to be individually evaluated to determine if it is actually desirable/useful for our code-base.

Describe alternatives you've considered We do not have to enable all of these checks if we determine that some are not useful/desirable. Not enabling any is also an option, though it just means that we are not taking advantage of validation that TS has to offer....

m5r commented 10 months ago

I've done some research previously and I found 2 complimentary approaches to incrementally migrate the project to strict mode.

We can use https://github.com/allegro/typescript-strict-plugin to manually migrate the project to strict mode file by file. The files are easily recognizable from the // @ts-strict-ignore comment at the top of each file. Here is a large repo that is using this plugin: https://github.com/adobe/react-spectrum/issues/3183

Additionally, we could use https://github.com/airbnb/ts-migrate to automate bits of the migration like explicitly typing the untyped stuff with any. There is existing tooling to measure how many any are left to type.

Bonus note: I recently heard of Betterer and they have a detailed walkthrough on how to set it up to migrate to typescript strict mode. I haven't dug into it that much but it looks very interesting :eyes:

jkuester commented 2 weeks ago

Leaving a note to myself here. Did some digging into the eslint issues with pulling in external types (e.g eslint complains about PouchDB.Database). I think it is related to the fact that we are not specifying any project in our eslint override config for the TS files.

      parserOptions: {
        project: 'tsconfig.json',
      },

Simply adding that to the override caused new errors (around linting files not actually included in the project), but seems like it might be the right direction to go...