Rewrite shell module loader in C++
Categories
(Core :: JavaScript Engine, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox78 | --- | fixed |
People
(Reporter: jonco, Assigned: jonco)
References
Details
Attachments
(1 file)
Originally I wrote the shell module loader in self-hosted JS because I thought that this would ease development for what was going to be a large component that underwent many changes as the spec developed. In the end the spec went with a minimum viable product approach though and many of the features envisioned were never added.
A JS module loader constrains our engine implementation because we must support script private values that are JS objects (for the shell) as well as reference counted C++ objects (for the browser). Also, the self-hosted code did not turn out as clean and readable as hoped because of the constraints of that environment (e.g. having to call methods with ReflectApply). Finally, writing the module loader in JS requires a ton of hook functions in the shell that only exist for that purpose.
Therefore I think we should replace the shell module loader with a C++ version.
Assignee | ||
Comment 1•4 years ago
|
||
Sorry for the big patch. This is a straight rewrite of shell/ModuleLoader.js in C++. It's mostly straigtforward but there were a couple of clunky parts: using promises/closures from C++ was rather verbose and I had to write some string utilities.
Updated•4 years ago
|
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/bd25dceab620 Rewrite the shell module loader in C++ r=jandem
Comment 3•4 years ago
|
||
Backed out for Hazzard bustages.
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=302468163&repo=autoland&lineNumber=9144
Backout: https://hg.mozilla.org/integration/autoland/rev/1acf50c375418804e9e646f8154ad3bd8d97ecce
Pushed by jcoppeard@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c2a97b6be526 Rewrite the shell module loader in C++ r=jandem
Comment 5•4 years ago
|
||
bugherder |
Assignee | ||
Updated•4 years ago
|
Description
•