1525652 - Convert DevTools modules to ES Modules

Status: NEW

(DevTools :: Framework, enhancement, P3)

Description

[Alexandre Poirot [:ochameau]]

This is a significant goal to have, but we should investigate how we can start loading at least a few independent modules via ES Modules instead of our custom module loader.
We can start with the few modules in devtools/server/ or devtools/shared/transport that do not pull any other module and are used from a single callsite.

The three main issues behind loading DevTools Modules as ES Modules are:

• for now, import() is gated by a pref only enabled on nightly. It means that if this preference doesn't ride the train, any code relying on import will break on beta.
• import() is asynchronous and returns a promise. It means propagating the promise to every call-site importing a symbol. As we would incrementally start loading just a few modules, it means that the few call-site we will convert would have to switch from synchronous require(...) to async import(...). This may significantly complexify the codebase
• import() seems to be only designed to be working from a document context. You have to have a document to have a working import() function. But many DevTools modules aren't related to any document. Like JSMs, they are singleton, whose lifetime isn't related to any particular document. Nor do they require any access to a document/DOM.

The reasons to switch to ES Modules are various:

• Get rid of custom DevTools modules loader, inherited from the Addon SDK. And so get rid of the usage of loadSubScript which is hard to maintain from what I heard from the JS team. And it also uses various tricky codepath in the JS engine which may make the devtools modules slower to execute.
• Switch from CommonJS to a Web Standard: ES Modules.
• Dogfood Firefox's ES Modules implementation.
• More easily share code between DevTools and the rest of Firefox. For now, you have to use our module loader to use any of DevTools code.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1525652 - Experiment with loading EventEmitter module as a ES Module.

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 9041865 [details]
Bug 1525652 - Experiment with loading EventEmitter module as a ES Module.

Jon, I was wondering what is you opinion on this.

Especially about the first comment of this bug.
This patch is obviously very hacky, due to its usage of processNextEvent...

I think it is OK to use a window-less browser for DevTools.
We may benefit from using a document and I don't think there is any significant drawback to use one other than using more resources than a Sandbox...
There the principal story to be carefully reviewed. The compartment has to be correctly flagged with invisibleToDebugger=true.

Otherwise, I think that the main issue is the fact that import() is asynchronous.
It makes it hard to:

• incrementally switch to ES Modules
• have lazy loading with ES Modules
  As it forces to be importing things via a promise instead of using a global symbol right away.

May be the subject of lazy loading has been already raised at the spec level? Or in the discussions to migrate JSMs to ES Modules?

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Alexandre Poirot [:ochameau] from comment #0)

The three main issues behind loading DevTools Modules as ES Modules are:

• for now, import() is gated by a pref only enabled on nightly.

This will ride the trains, hopefully soon. We're aiming for 67 or 68.

• import() is asynchronous and returns a promise.

For ES modules your options are static eager |import| in a module or dynamic asynchronous |import()|. There's no synchronous dynamic loading. But I think we could provide some system function that did this under the hood, along the lines of your patch here (although this is rather outside my area of knowledge).

• import() seems to be only designed to be working from a document context.

Yes, loading is implemented in the ScriptLoader which is owned by a document. There's no way around this at the moment, although for worker modules we will have to disentangle module loading from the (main) ScriptLoader.

May be the subject of lazy loading has been already raised at the spec level? Or in the discussions to migrate JSMs to ES Modules?

There are no plans to add this to JS. It has certainly come up in relation to JSMs which also have this problem and was being explored in bug 1432901.

Comment 4

[Alexandre Poirot [:ochameau]]

Bug 1602931 might be a blocker because CommonJS, without caching may be slower.
Bug 1432901 is the equivalent of this bug, but applied to JSMs.