1929387 - Migrate Reps require.js/AMD/CommonJS modules to ESM

Status: RESOLVED FIXED

(DevTools :: General, task)

Description

[Alexandre Poirot [:ochameau]]

A prerequisite to migrate JSON Viewer to ES Modules would be to migrate the Reps modules to ESM.
These modules are used from various places in very different ways.
JSON Viewer loads them via Require.js and the rest of DevTools via the CommonJS Loader.

Migrating Reps to ESM would ease using them from some other places, like the Profiler frontend to show JS tracer object previews.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Expose ES Module versions for React modules.

Land ES Modules for all React modules.

Also tweak the Browser Loader to force loading react loaded via require() to use the ES Modules.
This helps ensure these modules are loaded only once and always as a ES Module.
Otherwise require() statements from debugger frontend and import from Reps may load
distinct instances.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert Reps to ES Modules.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Fix existing eslint issues around reps.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert JSONViewer to ES Modules.

This get rid of intermediate viewer-config as we can now load json-viewer.js directly as a module
from the converter module.
And also Require.JS!

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert Tree shared component to ES Modules.

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert focus to ES Module.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert scroll module to ES Module.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Convert Tabs modules to ES Module.

Comment 9

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Trying to fix reps node tests

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Try to migrate jest to mochitest

Comment 11

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Remove all debugger node tests.

Comment 12

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Remove all Reps tests.

Comment 13

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Remove object-inspector tests.

Comment 14

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Remove other node tests involving reps.

Comment 15

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Fix ES Module loading when the JSON in transmitted in chunks.

Comment 16

[Phabricator Automation]

Comment on attachment 9439724 [details]
Bug 1929387 - [devtools] Remove all debugger node tests.

Revision D230118 was moved to bug 1933832. Setting attachment 9439724 [details] to obsolete.

Comment 17

[Alexandre Poirot [:ochameau]]

I'm getting closer and closer, but still some failures on DEBUG builds:
https://treeherder.mozilla.org/jobs?repo=try&selectedTaskRun=H2b_LoG8Qzy6VKq0Uo6jCw.0&revision=3b5100c85d5d3a09d7ec7a281164b0b28a5ad9b2

Comment 18

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Avoid loading duplicated debugger modules instances.

This also helps the esm-ification of Reps as it ensures loading
all debugger module in the debugger document global.
Reps modules are expected to be loaded with a document as global scope.

Comment 19

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Ensure loading all callsites of ObjectInspector via a BrowserLoader.

This is required as Object Inspector has to be loaded via a document scope
for its ChromeUtils.importESModule + global="current" to be functional.

Comment 20

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Remove leftover worker tests in debugger node tests.

Comment 21

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Allow mjs for react modules in dom parser

Comment 22

[Phabricator Automation]

Comment on attachment 9465436 [details]
Bug 1929387 - [devtools] Avoid loading duplicated debugger modules instances.

Revision D237738 was moved to bug 1949437. Setting attachment 9465436 [details] to obsolete.

Comment 23

[Phabricator Automation]

Comment on attachment 9437717 [details]
Bug 1929387 - [devtools] Expose ES Module versions for React modules.

Revision D228995 was moved to bug 1949486. Setting attachment 9437717 [details] to obsolete.

Comment 24

[Phabricator Automation]

Comment on attachment 9465439 [details]
Bug 1929387 - [devtools] Allow mjs for react modules in dom parser

Revision D237741 was moved to bug 1949486. Setting attachment 9465439 [details] to obsolete.

Comment 25

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Prevent sharing default props objects between TreeView instances.

The previous setup was spawning the TreeView once per DOM Panel instantiation,
so that defaultProps would be shared across all subsequent instances of TreeView
for a given DOM Panel instance.
The new setup makes it now shared across toolbox restart and makes
the expanded node be remembered across toolbox reboot.

But we were actually reliable on that unexpected sharing of expandedNodes Set
on TreeView props... so I had to ensure setting a stable expandedNodes props
from DomTree so that it isn't reset on each new receivingProps!

Comment 26

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Rename Reps modules to .mjs.

Comment 27

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Rename JSONViewer modules to .mjs.

Comment 28

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Rename Tree shared component modules to .mjs.

Comment 29

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Rename scroll module to .mjs.

Comment 30

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1929387 - [devtools] Rename Tabs module to .mjs.

Comment 31

[Alexandre Poirot [:ochameau]]

A quick performance overview of latest revisions:
https://perf.compare/subtests-compare-results?baseRev=095895327aa009d675e0f4caa10bdf14eb3e85aa&baseRepo=try&newRev=1b85d11245cd5a702d66f45cba24b5df6cb4a6a7&newRepo=try&framework=12&baseParentSignature=4763542&newParentSignature=4763542&filter_confidence=medium%2Chigh&sort=delta%7Cdesc

3% and 2% improvements on custom.netmonitor.manyrequests.requestsFinished and console.bulklog. No immediate idea on why these two are faster. Could it be that JS is faster to run when loaded from a true ESM scope rather than from DevTools sandboxes/require.js?

Otherwise some regression on all console open tests. This may be related to me removing a couple of lazy loading here and there. I'll scan the patch queue and try to revive all existing lazy loading.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a3352e22a716
[devtools] Convert Reps to ES Modules. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/fc1a3cf0a47c
[devtools] Rename Reps modules to .mjs. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/4553baa3eba9
[devtools] Remove all Reps tests. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/ada5414f55e4
[devtools] Remove object-inspector tests. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/9e50f94f5b47
[devtools] Remove other node tests involving reps. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/22f3cf41320b
[devtools] Fix existing eslint issues around reps. r=frontend-codestyle-reviewers,devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/9530ca3e17ae
[devtools] Convert JSONViewer to ES Modules. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/e0745e7311da
[devtools] Rename JSONViewer modules to .mjs. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/445379536388
[devtools] Convert Tree shared component to ES Modules. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/4ef4e168676e
[devtools] Rename Tree shared component modules to .mjs. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/d6f8fd15b030
[devtools] Convert focus to ES Module. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/e3d42cccf18d
[devtools] Convert scroll module to ES Module. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/0d948d0b5689
[devtools] Rename scroll module to .mjs. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/c1694f4e8c84
[devtools] Convert Tabs modules to ES Module. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/bb15fa7a6f09
[devtools] Rename Tabs module to .mjs. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/462f7d055427
[devtools] Fix ES Module loading when the JSON in transmitted in chunks. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/25a46f2f433d
[devtools] Ensure loading all callsites of ObjectInspector via a BrowserLoader. r=devtools-reviewers,nchevobbe
https://hg.mozilla.org/integration/autoland/rev/3d2bab518efa
[devtools] Prevent sharing default props objects between TreeView instances. r=devtools-reviewers,nchevobbe

Comment 33

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/a3352e22a716
https://hg.mozilla.org/mozilla-central/rev/fc1a3cf0a47c
https://hg.mozilla.org/mozilla-central/rev/4553baa3eba9
https://hg.mozilla.org/mozilla-central/rev/ada5414f55e4
https://hg.mozilla.org/mozilla-central/rev/9e50f94f5b47
https://hg.mozilla.org/mozilla-central/rev/22f3cf41320b
https://hg.mozilla.org/mozilla-central/rev/9530ca3e17ae
https://hg.mozilla.org/mozilla-central/rev/e0745e7311da
https://hg.mozilla.org/mozilla-central/rev/445379536388
https://hg.mozilla.org/mozilla-central/rev/4ef4e168676e
https://hg.mozilla.org/mozilla-central/rev/d6f8fd15b030
https://hg.mozilla.org/mozilla-central/rev/e3d42cccf18d
https://hg.mozilla.org/mozilla-central/rev/0d948d0b5689
https://hg.mozilla.org/mozilla-central/rev/c1694f4e8c84
https://hg.mozilla.org/mozilla-central/rev/bb15fa7a6f09
https://hg.mozilla.org/mozilla-central/rev/462f7d055427
https://hg.mozilla.org/mozilla-central/rev/25a46f2f433d
https://hg.mozilla.org/mozilla-central/rev/3d2bab518efa