Closed Bug 1637529 Opened 1 year ago Closed 1 year ago

Rewrite shell module loader in C++

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
mozilla78
Tracking Status
firefox78 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

(Regressed 1 open bug)

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.

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.

Severity: -- → N/A
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bd25dceab620
Rewrite the shell module loader in C++ r=jandem
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c2a97b6be526
Rewrite the shell module loader in C++ r=jandem
Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla78
Flags: needinfo?(jcoppeard)
No longer regressions: 1640298
Regressions: 1644760
You need to log in before you can comment on or make changes to this bug.