feat: review and update tsconfig [#1445]
This MR is a result of analysing our tsconfig and making adjustment.
tsconfig analysis
compileOnSave: false ➡ removed.
This field does not exist in the TSConfig Reference. I see it sometime mentioned in a SO thread but it seems to be an option only for Visual Studio (not VSCode).
compilerOptions.allowJs: true ➡ keep.
Allows importing JS file into TS file. We definitely need this.
I tried to also add checkJS to see what would be the result and we get 298 errors in 17 files (seems to all be "implicit any" tho), also briefly experimented with adding a separate tsconfig file for js files but ran into some problems. All of this to say: let's not add checkJS: true as part of this MR.
compilerOptions.allowUnreachableCode:false ➡ keep.
Raises error instead for unreachable code. That's good, it's a better config than the default that only warns.
compilerOptions.esModuleInterop:true ➡ keep.
When typescript initially implemented es6 modules, they were not very well supported in the JS ecosystem, so they took some decisions on how to convert them to CommonJS import. They ended up in a situation where typescript import did not comply with the ES6 module specs, and setting esModuleInterop:true fixes that. You can read more about the details here.
compilerOptions.forceConsistentCasingInFileNames:true ➡ keep.
Solves the problem of some OSs resolving filepath when casing is not matching, and other not doing it (docs). That's good.
compilerOptions.module:es2020 ➡ keep. (should be changed to es2022)
This is a big one. There is a lot to know to understand if this is the config we want or not. Typescript docs have a lenghty, multiple pages, well written documentation going over this. You can also skip directly to this part of the documentation or this part that are the most relevant for choosing the config.
The TL;DR is:
- We are targeting browser enviornement, so we want a flavour of
es - Our transpiling currently does not support top level await, so we should use
es2022oresnextinstead ofes2020
If we update our transpiling pipeline to latest then we can move to es2022 or esnext. Since updating the transpiling pipeline is out of scope for this MR, we can keep the current config confidently.
compilerOptions.moduleResolution:node ➡ keep (should be changed to bundler).
That's the other "big one". This config basically lets typescript know how we are resolving file paths when importing. File paths resolution depends on which environment the transpilation will be run in, modules are not resolved exactly the same way in the browser, in nodeJS or other JS runtime such as Deno, Bun or Cloudflare Workers. This is also not mentioning bundlers: when the transpilation is done by a bundler, it's possible to configure them to do things not supported by JS runtimes, such as using import for assets like images or css/scss files.
Our case is that we use a bundler, transpilation is done by ts-loader (not tsc) which is invoked by webpack, so we should use the configuration compilerOptions.moduleResolution: "bundler". However this configuration is only available in typescript 5, and i want to keep upgrading typescript major version out of scope for this MR (I did give it a shot and ran into some problems, albeit minor and I think the upgrade should be fairly straightforward).
Right now we could use node16 module resolution, which was introduced with typescript 4.7 (the version we use), and is essentially saying "match the module resolution of the ES6 Module specs". However trying it I did get errors relative to importing some .mjs files. These errors may or may not be solved by adding type: "module" to our package.json but we can't do that because we also still use require() (CommonJS) in many frontend files.
In conclusion we keep things this way and will create follow up tasks for bringing the tooling/code up to date so we can use bundler for module resolution.
compilerOptions.removeComments:true ➡ keep.
Not much to say here. We want to strip out comments when transpiling.
compilerOptions.skipLibCheck:true ➡ keep.
This is an interesting one: what this is doing is skipping typechecking of .d.ts files. Right now we have several librairies with errors in their typedef files, namely: mocha, jest and hyperhtml-element, which I assume are why this was enabled in the first place. However we also have manually created some .d.ts files and turns out, the file with the typedefs of the ewe SDK (src/webext-sdk.d.ts) is choke full of syntax errors, but because skipLibCheck is enabled, this file is not being validated.
By skipping typedef files, we are loosing type-system accuracy. We should fix this problem, however I keep it out of scope of this MR and will create a follow up issue instead.
compilerOptions.strict:true ➡ keep.
Also not much to say here. Turning this to true makes typescript a lot more valuable. If you're not aware of this option then read the docs.
compilerOptions.target:es2016 ➡ change to es2018.
To understand what we should use here, we have to understand what kind of browser support we're aiming for. Our current target is:
chrome >= 77
edge >= 77
firefox >= 63
opera >= 64The current configuration, es2016 (a.k.a ES7) is equivalent to es2015 (a.k.a ES6) + two extra features.
ES6 Support is largely within the range, and the two extra features added by ES7 are also supported by all browsers we target.
So can we bump that number up a bit? If we look at es2020 we can see that we would have to drop support for a few versions for all 4 browsers, so that's to high (scroll at the bottom of this page for the table)
Let's look at es2018 (again, the can I use page for the support tables of es2018/ES8 features)
So here we can see that ES8/es2018 is supported by:
chrome >= 58 (<= 77 ✅)
edge >= 15 (<= 77 ✅)
firefox >= 52 (<= 63 ✅)
opera >= 45 (<= 64 ✅)Thus, we can safely update our target to es2018/ES8, which may result in some bundle size reduction since not as many JS features need to be polyfilled anymore.
This will make QA for this MR more involved, we should make sure the extension generated by the build indeed works fine in older browsers.
compilerOptions.typeRoots:["node_modules/@types"] ➡ keep
This prevents typescript from looking for looking for typedefs in parent node_modules directories that would be outside of the project (cf. docs).
If we every manage to setup a mono repository, we might to change this, but for now it's a good config to keep.
"include": ["src/**/*"] keep
We only want type checks to run inside the src directory and nowhere else. Nothing to change here.
Potential additions
lib (docs)
It's a commonly seen config, this lets typescript know which typings we want to be included by default. For an example, when writing code that only runs in NodeJS, one wouldn't want typings for DOM apis to be included because window or document won't exist in the global scope, but instead one would like to have typing for process in the global scope.
Typescript already does includes some things and exclude others depending on the compilerOptions.target value, and that is good enough for us at this point
We already have the @types/webextension-polyfill package that adds all the typings we need for web extension development and it is being correctly picked by typescript.
noEmit:true
tsloader also uses the tsconfig.json file, so if we set noEmit to true, then we won't get any transpiling done. If we ever switch to a faster transpiler that does not use the tsconfig file (like swc) then we'll want to set this option to true.
resolveJsonModule:true
If we want to be able to import JSON files using es6 syntax, might come in handy. Let's add it if/when we need it. Right now we have this getJSON() function that we mainly use at build time to build the manifest file, we could remove it and just import JSON files directly instead.
Other
I checked a few other "famous" webext config files and see if we can draw some inspiration from them:
Also looked at uBlock, react-devtools but they don't use TypeScript
Only differences I could see is that they are more explicit about their lib config and then some variations in opinionated typechecking settings such as noPropertyAccessFromIndexSignature or noUncheckedIndexedAccess.
TODOs:
-
MR to update typescript and the transpiling pipeline in general (i.e. babel) + switch to moduleResolution: bundler -
Create follow up issue for enabling skipLibCheckand fixing related problems.
Related to #1445 (closed)