1766761 - Provide a shim for ESM-ified modules

Status: RESOLVED FIXED

(Core :: XPConnect, task)

Description

[Tooru Fujisawa [:arai]]

There are still not-in-tree consumer of Cu.import for JSM in AutoConfig (see bug 1766114),
and to make the transition easier, it's better providing a shim that makes Cu.import(".../foo.jsm") keeps working even after foo.jsm is converted to ESM (foo.mjs), until all modules are converted.
So that people can rewrite their AutoConfig script at once.

The shim may also help the in-tree transition itself, given the transition can be done per file, not subtree.

Possible solutions are:

• (a) Once foo.jsm is converted to foo.mjs, add foo.jsm that imports foo.mjs and export the same symbols with existing JSM semantics
• (b) Modify Cu.import internal (somewhere around mozJSComponentLoader) to automatically redirect to foo.mjs when called with foo.jsm, for already converted modules

Comment 1

[Tooru Fujisawa [:arai]]

If we take (b), the change needs to be applied onto bug 1311728 patch

Comment 2

[Tooru Fujisawa [:arai]]

Other possible solution is:

• (c) If foo.jsm is not found, automatically redirect to foo.mjs, and if foo.mjs exists, convert the call to importModule and continue

The following code checks the existence of the actual file, then-clause for resource:// and else-clause for chrome://.
So, if the redirect is still possible at that point, we could take this way.
This doesn't require manual registration or wrapper JS file, and the transition becomes easier.

https://searchfox.org/mozilla-central/rev/a730b83206183bf097820c5780eef0d5e4103b9d/js/xpconnect/loader/mozJSComponentLoader.cpp#795-808

nsresult mozJSComponentLoader::ObjectForLocation(
    ComponentLoaderInfo& aInfo, nsIFile* aComponentFile,
    MutableHandleObject aObject, MutableHandleScript aTableScript,
    char** aLocation, bool aPropagateExceptions,
    MutableHandleValue aException) {
...
    if (realFile) {
      AutoMemMap map;
      MOZ_TRY(map.init(aComponentFile));
...
    } else {
      nsCString str;
      MOZ_TRY_VAR(str, ReadScript(aInfo));

Comment 3

[Tooru Fujisawa [:arai]]

For options (b) and (c), Cu.isModuleLoaded and Cu.unload may need to handle the redirected case.

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1766761 - Part 4: Add a shim for Cu.import/ChromeUtils.import with ESM-ified module. r?jonco!,Standard8!

Comment 5

[Tooru Fujisawa [:arai]]

the attached patch is for (c) way.
it automatically redirects import for JSM if ESM-ified.
the return value is a proxy that wraps ModuleEnvironmentObject, to make it read-only and hide *namespace* binding.
It adds a hacky workaround that de-optimizes the top-level var so that they're stored in ModuleEnvironmentObject, not the local slot.
This can affect the performance, but doesn't regress given global variables in JSM is also not optimized.

Comment 6

[Tooru Fujisawa [:arai]]

Here's Cu.import return value usage in Thunderbird/SeaMonkey extensions compatible with recent versions.
All symbols are exported. so they won't be affected by the migration as long as
they're not converted to lexical binding before ESM-ified.

module URI	symbols	binding type
resource://gre/modules/NetUtil.jsm	NetUtil	global var
resource://gre/modules/FileUtils.jsm	FileUtils	global var
resource://gre/modules/Services.jsm	Services	global var
resource://gre/modules/AddonManager.jsm	AddonManager	global var
resource://gre/modules/Console.jsm	console	global var
resource://gre/modules/FileUtils.jsm	FileUtils	global var
resource://gre/modules/PluralForm.jsm	PluralForm	global var
resource://gre/modules/LightweightThemeManager.jsm	LightweightThemeManager	global var
resource://gre/modules/XPCOMUtils.jsm	XPCOMUtils	global var
resource://gre/modules/AsyncShutdown.jsm	AsyncShutdown	global var
resource://gre/modules/Console.jsm	console	global var
resource://gre/modules/AppConstants.jsm	AppConstants	global this property
resource://gre/modules/osfile.jsm	OS	global this property
resource:///modules/MailUtils.js	MailUtils	global var
resource://devtools/shared/Loader.jsm	loader, require	global var

Comment 7

[Tooru Fujisawa [:arai]]

bug 1768060 patch adds support for lexical variables in Cu.import return value, so, if we can go with it, converting var or this.foo = ... with lexical won't break not-in-tree code

Comment 8

[Tooru Fujisawa [:arai]]

Attachment: Bug 1766761 - Part 1: Add compile option to de-optimize module global variables. r?jonco!

This is necessary for putting all global vars in ModuleEnvironmentObject,
instead of local slot, so that they're accessible in Part 4 patch.

Comment 9

[Tooru Fujisawa [:arai]]

Attachment: Bug 1766761 - Part 2: Rename shell GetModuleEnvironment function. r?jonco!

In order to add public API that returns module's environment, shell-only
function GetModuleEnvironment needs to be renamed.

Depends on D146033

Comment 10

[Tooru Fujisawa [:arai]]

Attachment: Bug 1766761 - Part 3: Add public API for getting module from namespace, and getting module environment. r?jonco!

Depends on D146034

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/435acc6d6abd
Part 1: Add compile option to de-optimize module global variables. r=jonco
https://hg.mozilla.org/integration/autoland/rev/d8099b6d970b
Part 2: Rename shell GetModuleEnvironment function. r=jonco
https://hg.mozilla.org/integration/autoland/rev/a7a37e912b90
Part 3: Add public API for getting module from namespace, and getting module environment. r=jonco
https://hg.mozilla.org/integration/autoland/rev/682c4afbcfe9
Part 4: Add a shim for Cu.import/ChromeUtils.import with ESM-ified module. r=jonco,Standard8

Comment 12

[Cristian Tuns]

Backed out for causing spidermonkey failures on Modules.cpp

• Backout link
• Push with failures
• Failure Log
• Failure line: /builds/worker/checkouts/gecko/js/src/vm/Modules.cpp:237:10: error: cannot initialize return object of type 'JSObject *' with an rvalue of type 'js::ModuleEnvironmentObject *'

Comment 13

[Tooru Fujisawa [:arai]]

there was missing include.
I'll fix it

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/35d2a337b15e
Part 1: Add compile option to de-optimize module global variables. r=jonco
https://hg.mozilla.org/integration/autoland/rev/30aecbe14c23
Part 2: Rename shell GetModuleEnvironment function. r=jonco
https://hg.mozilla.org/integration/autoland/rev/5811c45ba29c
Part 3: Add public API for getting module from namespace, and getting module environment. r=jonco
https://hg.mozilla.org/integration/autoland/rev/651fcbcabee5
Part 4: Add a shim for Cu.import/ChromeUtils.import with ESM-ified module. r=jonco,Standard8

Comment 15

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/35d2a337b15e
https://hg.mozilla.org/mozilla-central/rev/30aecbe14c23
https://hg.mozilla.org/mozilla-central/rev/5811c45ba29c
https://hg.mozilla.org/mozilla-central/rev/651fcbcabee5