1522042 - Load debugger modules as ES Modules instead of CommonJS

Status: NEW

(DevTools :: Debugger, task, P3)

Description

[Alexandre Poirot [:ochameau]]

For now, debugger modules are written as ES Modules, but the build step (copy-module.js + babel) convert them to CommonJS in order to be loaded via the DevTools module loader.

It would allow:

• dogfooding es module implementation in Firefox and help JS engine team to uncover possible issues in our implementation
• simplify debugger build step by possibly limiting it to translate only JSX/Flow to JS
• reduce the need of source map support for debugging the debugger as only JSX/Flow pieces will be converted
• may be speed up the debugger build step
• open ways to drop the very custom DevTools loader and use web standards!
• no longer depends on loadSubScript and possibly have faster JS with less non-standard behavior around scopes

There is a couple of challenges here:

• React and most vendored library aren't designed to be loaded as ES Modules in the browser and so have to be loaded as CommonJS or <script> tag (is there any other way to load them?)
• All external dependencies made to devtools modules from mozilla-central have to still use the DevTools module loader as they are still commonjs modules
• require paths have to be explicit. Browser's import doesn't support expending to index.js when refering to a folder, like "import * from ./folder/" has to be "import * from ./folder/index.js"
• import lines have to be written before any JS code in modules, which would require a special refactoring in order to keep this rule true.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1522042 - Load Debugger modules as ES Modules instead of CommonJS.

• React is not designed to be loaded as an ES Module in the browser.
  It seems to always assume going through babel or some converter in order to be loaded as commonjs or <script>.
  So load it as a <script> in order to be loaded in the same compartment than the document.
  Even if, in theory, thanks to latest work on compartments, all are shared.
  I did this for React and all other vendored libraries.

• copy-module.js is tweaked to:

  • convert react import to fetch symbols out of window.React. (Do the same for all libraries)
  • In ES Modules import have to be done first before anything else, so it ensures that rule by sorting the header of all js files
  • ignores the "import file.css" that I don't know how to translate nor why we do that.
  • still convert to require import that relates to devtools modules that have to be loaded via require()
  • handle the implicit "index.js" when doing import * from "./folder/"
  • I may miss some other action?

There is opportunities to move forward incrementaly here:

• convert React and other library to a non-loader based solution (I can only think about <script> tags, but that would be great if we can load them as es module...)
• refactor implicit index.js and always require explicit paths
• may be switch to require the imports that have to go through require?
• sort the import, have the one that can stay real import first so that we don't have to reorder at build time.
• reduce the number of custom magic modules like devtools-components
• try to migrate to es modules things that still depends on commonjs (loadash, vendored, reps)

Basically, all the incremental steps would be to drop parts of copy-module.js.
The goal to switch to es module is basically a way to drop most of copy-module.js in order to only keep JSX and Flow.

MozReview-Commit-ID: LY4yGPwrhf4

Comment 2

[Alexandre Poirot [:ochameau]]

Note that bug 1513014 makes it very hard to debug issues related to incorrect import path.
Most of the error are completely silent, making it really hard to know if/where there is an issue :/

Comment 3

[Alexandre Poirot [:ochameau]]

Try run, which highlights a couple of broken tests still:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=f5dbbb7c7b7aa0df98149bdff41f5b31da9cce78

And DAMP, which highlights improvements and regressions at the same time:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=07b39f88594ea3921f87c858ce31c35e5e1729d4&newProject=try&newRevision=f5dbbb7c7b7aa0df98149bdff41f5b31da9cce78&originalSignature=1759151&newSignature=1759151&showOnlyConfident=1&framework=12
I'm looking forward rebasing this on top bug 1517210 in order to see how the sharing compartments impact this work.
In a world where all compartments are shared, switching from DevTools loader to ES Module shouldn't change anything regarding wrappers/compartments. But today, we are still having some wrappers and distinct compartments. Bug 1517210 is aiming at getting rid of them, finally.

Comment 4

[Alexandre Poirot [:ochameau]]

I rebased this patch on top of WIP patch queue out of bug 1517210 (to merge all the compartments) and compared against it:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=17fa3281553dbbb49741e987e85a1fbfe9bd9ab6&newProject=try&newRevision=681f1a6d86b55a656ed9cb58c0295a44056b3746&originalSignature=1759151&newSignature=1759151&showOnlyImportant=1&framework=12
It looks like this patch slow down things... It can be es modules, or something broken that makes DAMP slower, or loading the libraries via script tags, ...?

Comment 5

[Ted Campbell [:tcampbell]]

The startupcache doesn't support ES Modules which is probably hurting loading perf. Bug 1588861 will get us closer.