Enforce and fix typescript lints

[atjn]

@bobrippling disabled linting for typescript projects in #3383. I don't understand why typescript projects should be exempt from the linting rules, and I could not find any explanation in the PR either.

I propose to reenable linting rules for typescript files and fix the lint issues, as done in this PR. I have also checked that none of the project files are temporarily exempt, so that means these few changes are enough to fix all lint issues in all typescript projects.

[thyttan]

I merged the change. I believe the reasoning is that since the js files were generated from the ts source the warnings should not (never?) correspond to a real problem.

What prompted the change: https://github.com/espruino/BangleApps/pull/3383#discussion_r1582617557

But I'll let you @atjn and @bobrippling talk this out more, I'm hardly familiar with these things.

[atjn]

I merged the change. I believe the reasoning is that since the js files were generated from the ts source the warnings should not (never?) correspond to a real problem.

That is a fair assumption, but the linter is testing for many things that typescript does not. They complement each other well :)

It would be possible to add plugins to the linter so that it checks the typescript files directly, but it really doesn't matter whether the js files or ts files are linted, as long as one of them is.

What prompted the change...

Thanks for the pointer. Given that I have fixed that issue, I am guessing there is no problem now?

[atjn]

Thanks for the reply @bobrippling. I did not think about the case where you declare a global variable in a type-safe manner. To support that case, the linting needs to happen in the ts file, and the output js file must be ignored because it lacks the type information that makes it okay to use that variable.

Therefore I have completely remade the PR so that it now includes a typescript linter, just as you talked about. The new linter also has some ts-specific lint rules so I needed to fix some code. What do you think?

[bobrippling]

Nice! Yeah that all looks great, thank you.

One thing I'm interested in your opinion on - I think unlike the original lint changes where we wanted to bump app versions when things like variable lets were introduced, here we can confirm that some apps have no effective changes, for instance:

• the corresponding .js file is unchanged
• the changes (in the output js) are trivially bug-free, e.g.:
  • addition of scope-brackets (and no block-scoped variables affected)
  • o.a -> o["a"]

So I would say we can avoid the version bump in these cases. There's just the Array -> [...] change where we perhaps keep the version bump out of caution, or perhaps that me being too cautious, what do you think?

[atjn]

Very nice!

For apps where there is no change to the output, I have not bumped the version.

As for the other cases you mention, I bumped them out of caution just because there can be minor differences in their behavior and that is especially true in Espruino which does not always conform 100% to JS standards. If you request it, I don't mind unbumping them (realistically the changes are probably fine), but I would personally just bump them to be safe. Also be aware that I have not tested them yet, I didn't want to spend time on that before I knew if you approved the changes. I'll probably have time in the next few days :)

[bobrippling]

Ah of course - great stuff. I think that's perfectly reasonable, let's stick with your approach in that case. And no rush :)

[atjn]

I have tested the apps as best I could and could not find any breakage. I believe this is ready to be merged :)

[bobrippling]

Cool - thanks again!