prona-p4-learning-platform / learn-sdn-hub

12 stars 6 forks source link

Finished: Enhance tsconfig/eslint in backend #170

Closed Tomtec331 closed 5 months ago

Tomtec331 commented 7 months ago

Currently the tsconfig used in the backend is quite basic. Several useful functionalities of typescript aren't even enable. Especially the "strict" option should always be used (like in frontend). Essentially we should bring the backend in line with frontend. This would prevent many easily checkable coding mistakes.

Luckily the strict option consists of eight independent flags. So we can enable them on after another and fix emerging errors:

After all flags are set and errors are fixed the flags can be replaced with strict: true (see: https://www.typescriptlang.org/tsconfig#strict)

Additional optional options (also used in frontend) are:

srieger1 commented 7 months ago

Speaking of strict... when I updated React to 18, we needed to disable strict in the frontend app. Maybe this can be reverted now that we thankfully have an updated toolchain and deps...

Tomtec331 commented 6 months ago

Adding all important flags and fixing the code is finished. I also included an enhanced eslint configuration which now checks types too.

See: https://github.com/prona-p4-learning-platform/learn-sdn-hub/blob/14ed37b19c8d6bfcec200c6bfa0140b64728bab8/backend/.eslintrc.cjs#L16

I personally used "strict-type-checked" while fixing the code but the recommended option should be enough. I haven't touched all files because I cannot reliably test the outcome. I just disabled all necessary eslint rules in these files and added a TODO:

The last two are handling the express API paths which result in lots of "any" types as we're dealing with incoming data. "Celebrate" is already used - the problem is that all the parsed types are lost again after the middleware has run and we still need to cast all elements into the corresponding type - which can lead to new issues.

Nonetheless the changes have already been merge into the "develop" branch and are ready for testing.

srieger1 commented 6 months ago

As mentioned, I started to fixup the leftover eslint-disables. Current state:

srieger1 commented 5 months ago

won't fix OpenStackProvider for now as currently unused in our environments, the rest is fixed in develop now since 124489b