Change default TypeScript behavior to strict: true (fix #41)

[edemaine]

This PR enables strict: true diagnostic errors from TypeScript (only when the "Display Errors" checkbox is checked), as requested in #41. Example:

But I'm having some trouble with this turned on and the variable actually gets used (so it doesn't get tree shaken away). It seems that TypeScript is somehow processing the same file twice, which leads to new errors:

It seems that src/components/editor/monacoTabs.tsx's MonacoTabs creates two models: the model at the top of the function and the model for each tab in tabs. I'm seeing the same source code in both the root model and a tab, which is presumably the source of the problem. Why this isn't a problem without strict mode, I'm not sure. @amoutonbrady Could you explain what's going on here? Could we remove one of the duplicates (presumably the top-level model?)?

[milomg]

This is an issue with files that don't have imports or exports and are then treated as a global script file. One trick might be to add a tsconfig flag to ignore output_dont_import.tsx?

[edemaine]

@modderme123 Thanks for the tips! I didn't realize it was the lack of import/export that was causing the problem, and that it's an existing issue with the playground. I'll move this to a separate issue (#104).

This PR is now ready for review.

[edemaine]

Here's yet another example of confusion that this PR will fix: solidjs/solid#972

[amoutonbrady]

Thanks!

[amoutonbrady]

Looks like this has one side effect, the default example shows an error :/

[edemaine]

Looks like this has one side effect, the default example shows an error :/

Ha, whoops; sorry I didn't notice that. It's a useful error message though: document.getElementById can be null which would cause a crash.

One natural way to write this line in TypeScript is this:

render(() => <Counter />, document.getElementById("app")!);

But maybe we want to avoid TypeScript-specific notation in our opening example -- that seems like a nice property to maintain. So then I'd suggest:

const app = document.getElementById("app");
if (app) render(() => <Counter />, app);

It's a little wordier, but is definitely safer. What do you think?

[milomg]

Oops, I pushed a commit before I saw this opting for the first one. I'll deploy in a minute

[edemaine]

No worries! ! is a good choice too.