1801082 - Throw an explicit error when ChromeUtils.import tries to load a sys.mjs file

Status: RESOLVED FIXED

(Core :: XPConnect, enhancement)

Description

[Julian Descottes [:jdescottes]]

If you try to use ChromeUtils.import to load a sys.mjs file, you most likely will get a stack trace such as

SyntaxError: export declarations may only appear at top level of a module at chrome://remote/content/shared/webdriver/ReferenceStore.sys.mjs:121
@/builds/worker/workspace/build/tests/xpcshell/tests/remote/marionette/test/xpcshell/test_evaluate.js:7:47
load_file@/builds/worker/workspace/build/tests/xpcshell/head.js:762:11
_load_files@/builds/worker/workspace/build/tests/xpcshell/head.js:779:10
_execute_test@/builds/worker/workspace/build/tests/xpcshell/head.js:569:14

Everytime this happens to me, I start investigating what I possibly messed up in the sys.mjs file. But that file is usually fine, the problem is that I should have used ChromeUtils.importESModule and not ChromeUtils.import.

It would be great if ChromeUtils.import could detect that we are loading a sys.mjs file and throw before attempting to load the module. Maybe even add an ESLint rule?

Alternatively I guess ChromeUtils.import could also transparently forward to ChromeUtils.importESModule if the path has a sys.mjs extension, but that would probably lead to more complicated cleanup to do down the road.

Comment 1

[Tooru Fujisawa [:arai]]

Throwing better error message sounds good.
I'll look into this.

Comment 2

[Tooru Fujisawa [:arai]]

Here's example output of WIP patch when module-related syntax error happens in ChromeUtils.import.

> ChromeUtils.import("resource://gre/modules/XPCOMUtils.sys.mjs")
Uncaught Error: ChromeUtils.import is called against a module script
  (resource://gre/modules/XPCOMUtils.sys.mjs).  Please use ChromeUtils.importESModule
  instead (SyntaxError: import declarations may only appear at top level of a module)
    <anonymous>
    getEvalResult resource://devtools/server/actors/webconsole/eval-with-debugger.js:242
    evalWithDebugger resource://devtools/server/actors/webconsole/eval-with-debugger.js:166
    evaluateJS resource://devtools/server/actors/webconsole.js:1131
    evaluateJSAsync resource://devtools/server/actors/webconsole.js:1022
    makeInfallible resource://devtools/shared/ThreadSafeDevToolsUtils.js:103

Comment 3

[Julian Descottes [:jdescottes]]

Looks very clear to me, thanks :arai!

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1801082 - Report better error message when ChromeUtils.import tries to import ESM. r?jonco!,jdescottes!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a7e25072383b
Report better error message when ChromeUtils.import tries to import ESM. r=jonco,jdescottes

Comment 6

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/a7e25072383b