1713640 - Add typescript checks for JS modules in /remote

Status: NEW

(Remote Protocol :: WebDriver BiDi, task, P3)

Description

[Julian Descottes [:jdescottes]]

We would like to use typechecking through TypeScript comments. They will be checked via a specific job similar to what is done for DevTools' performance-new panel.

We will use TypeScript annotations for BiDi specific code and potentially for shared modules? Meaning this would apply to remote/bidi and remote/shared

Resources:

• typescript documentation for performance-new: https://searchfox.org/mozilla-central/source/devtools/client/performance-new/typescript.md
• existing types: https://searchfox.org/mozilla-central/source/devtools/client/performance-new/@types
• devtools node test runner: https://searchfox.org/mozilla-central/source/devtools/client/bin/devtools-node-test-runner.js
• performance-new package.json tsc usage: https://searchfox.org/mozilla-central/rev/d3b116e554eb3546a95851e81f7ddc53d5b7e096/devtools/client/performance-new/package.json#6

Basically to support typescript and run it on try, we should add a package.json to that will be able to run tsc on the relevant folders. We will need to decide if we create a dedicated runner or if we reuse the devtools one.

Note that currently there is no integration with mach (see https://bugzilla.mozilla.org/show_bug.cgi?id=1673423 for initial effort on that front). I suggest to focus this bug on adding basic typescript support & types to /remote, and leaving integration improvements for followup bugs.

Comment 2

[Julian Descottes [:jdescottes]]

Poked at this today, there are several issues.

First typescript does not support custom extension names, meaning it can't be configured to read *.jsm files. See issue at https://github.com/microsoft/TypeScript/issues/10939

We can rename our JSM files to *js, it doesn't change anything behind the scenes as far as I know, just annoying.

Next, typescript is not very configurable to understand custom import syntax. It can understand different "presets" (module, commonjs, require etc...), but I didn't see any way to make it understand things such as ChromeUtils.import or XPCOMUtils.defineLazyModuleGetters. To workaround this issue in the performance panel, it seems the solution was to create a loader function https://searchfox.org/mozilla-central/search?path=&q=devtools%2Fclient%2Fperformance-new%2Ftypescript-lazy-load.jsm.js which allows to still write a "require" call that Typescript will be able to parse.

I guess we can do something similar, but that will make our codebase look very alien to the rest of mozilla-central (which is already the case for devtools). Short of that, I imagine we would be limited to same-file typechecks + basic type definitions.

Quick question for Julien: when typescript was initially suggested as something that could be used in mozilla-central, did your team spot those issues and maybe came up with solutions/workarounds? Are there other potential issues you have in mind? Thanks!

Comment 3

[Greg Tatum [:gregtatum]]

For .jsm files the solution was to name them .jsm.js, that way they have the proper extension, and the actual dynamic module loaders don't care about the extension.

As you stated, the lazy loaders were worked around by the createLazyLoaders function. This is documented here: https://searchfox.org/mozilla-central/source/devtools/client/performance-new/typescript-lazy-load.jsm.js#29

I also wrote up this document, but I'm not sure how up to date it is compared to our final solutions: https://docs.google.com/document/d/1EESWui3riJrfJ5fl5u1YpIz8IQOUd5npxvyieINJvxU/edit#heading=h.ijo46lmg6oot

Comment 4

[Julian Descottes [:jdescottes]]

Thanks for re-sharing the doc Greg, that's very helpful. Clearing the ni? for Julien.

Comment 5

[Julian Descottes [:jdescottes]]

Setup a meeting to review if we do it as P2 or P3 for m3