Strengthen eslint rules for code consistency and quality

[dianabarsan]

Describe the issue

Our very relaxed style guide alongside our current eslint rules don't provide sufficient support or guidance for contributors to produce code that is ready to be merged. Frequently, for first time contributors, code reviews are filled with style related change requests. This extends the time between initial PR submission and final acceptance, and can also be detrimental to morale.

Describe the improvement you'd like

Enable more eslint rules to make sure code is consistent and readable and reduce the number of style-related PR change requests. The list of eslint rules to be added are listed here: https://docs.google.com/document/d/17l3Klzt12aEAPOss61aJUVQbNUaeKGlQGQt3keKdW5Q/edit#heading=h.5wseysgk236q

The list is very long and likely will end up touching many lines in the codebase. To ease reviewing, we'll be using a dedicated branch where all eslint related PRs will be merged, with this branch getting finally merged to master after being acceptance tested.

Every PR merged into the "temporary" branch will be reviewed and will only be merged after all tests have passed (we could even consider enabling branch protection rules).

All rules will be added to the cht-core eslint config as a first step. After all rules are applied, we will update https://github.com/medic/eslint-config with the new rules and remove them from cht-core eslint config. Other repos using https://github.com/medic/eslint-config should follow with applying the new set of rules.

Additionally, the style guide page should be updated to describe enforced rules and cover aspects of the style guide which are not covered by eslint (for example camelCase for variables and snake_case for json properties).

Describe alternatives you've considered

Only update the style guide page to include comprehensive information about our coding standards and have everyone and their mother painstankingly read it.

[garethbowen]

@dianabarsan In our code style doc it says...

Omit the return keyword when the entire function definition fits on one line.

But we still have plenty of code that omits the return keyword for multi-line. I couldn't find an eslint rule which would solve this completely but this one is close: https://eslint.org/docs/latest/rules/implicit-arrow-linebreak (with the default value of "beside"). What do you think?

[garethbowen]

It'd be good to add the no-return-await rule. Currently the code has 222 errors for this rule.

[dianabarsan]

no-return-await was deprecated: https://eslint.org/docs/latest/rules/no-return-await

I actually prefer return await for this better call stacks:

Using return await inside an async function keeps the current function in the call stack until the Promise that is being awaited has resolved, at the cost of an extra microtask before resolving the outer Promise. return await can also be used in a try/catch statement to catch errors from another function that returns a Promise.

[garethbowen]

Interesting.

I think that quote is out of date now as in this blog post: https://v8.dev/blog/fast-async#improved-developer-experience - the stack trace includes the async call that threw the error even if it's not returned.

This is linked from the no-return-await page but as far as I can tell doesn't actually mention return await one way or the other.

Anyway, as it's deprecated, please ignore me!