For example this.LoginBreaches = { ... }: https://searchfox.org/mozilla-central/rev/ba4fab1cc2f1c9c4e07cdb71542b8d441707c577/browser/components/aboutlogins/LoginBreaches.jsm#25.

This should generally work by changing to const LoginBreaches = { ... }

Why don't you use globalThis.LoginBreaches = { ... }?
It is great to reduce global environment pollution, but is it really tied to migration to ES modules?

(In reply to Masatoshi Kimura [:emk] from comment #1)

Why don't you use globalThis.LoginBreaches = { ... }?
It is great to reduce global environment pollution, but is it really tied to migration to ES modules?

Not sure. Ted, do you have an opinion?

FWIW I have a script prepared to remove this that handles most cases I've found so far, although it's complicated by consumers that use Cu.import or ChromeUtils.import(path, null) which then expect things to have been set explicitly on this. I'm not sure if using globalThis will let those cases continue to work in the meantime, but if so that might be an option as a stopgap instead of waiting on those cleanups to happen first.

(In reply to Masatoshi Kimura [:emk] from comment #1)

Why don't you use globalThis.LoginBreaches = { ... }?
It is great to reduce global environment pollution, but is it really tied to migration to ES modules?

This would expose it across all modules I believe. The intent should be to keep modules self-contained and share via exports not on the shared global.

This patch was generated with the script https://github.com/bgrins/jsm-rewrites/blob/d2bbd6c459294b65955442e45b5a7f5dba11e639/no-this-property-assign.js
using the following command:

cp .gitignore .rgignore && rg -l -g '*.jsm' '' devtools | jscodeshift --stdin --transform ~/Code/jsm-rewrites/no-this-property-assign.js && ./mach eslint --outgoing --fix

There's also a manual fixup in Loader.jsm from const to var for a couple exports.

Yeah, using globalThis will expose to all modules. This seems like reasonable behaviour and is how things would be if you made a single-page-app with a bunch of ES modules.

(In reply to Dave Townsend [:mossop] (he/him) from comment #3)

(In reply to Masatoshi Kimura [:emk] from comment #1)

Why don't you use globalThis.LoginBreaches = { ... }?
It is great to reduce global environment pollution, but is it really tied to migration to ES modules?

This would expose it across all modules I believe. The intent should be to keep modules self-contained and share via exports not on the shared global.

Currently, modules are not self-contained because using this will expose things to shared JSM globals, i.e. across all modules. If the intent is to make modules self-contained, this bug should not block esm-ification. Because we can migrate to ES modules using globalThis. Although it is status quo, it does not make things worse.

We need to stop relying on the global this in order to support ES Modules.
This instance was harder to fix by a script than others since it exposed two methods,
so this patch is a manual change.

(In reply to Masatoshi Kimura [:emk] from comment #6)

(In reply to Dave Townsend [:mossop] (he/him) from comment #3)

(In reply to Masatoshi Kimura [:emk] from comment #1)

Why don't you use globalThis.LoginBreaches = { ... }?
It is great to reduce global environment pollution, but is it really tied to migration to ES modules?

This would expose it across all modules I believe. The intent should be to keep modules self-contained and share via exports not on the shared global.

Currently, modules are not self-contained because using this will expose things to shared JSM globals, i.e. across all modules. If the intent is to make modules self-contained, this bug should not block esm-ification. Because we can migrate to ES modules using globalThis. Although it is status quo, it does not make things worse.

It was my understanding that modules are self-contained and this is not shared across JSMs. I believe that in the B2G days we had a memory saving by using the same global for all JSMs which is why we started assigning things to this which was made unique for each JSM but I think even that has gone away now. In tests I do not seem to be able to access anything from a different module from the one currently running.

A top-level this in a JSM is isolated to just that JSM, while globalThis is not. In ES modules, a top-level this is undefined and globalThis is shared.

We need to stop relying on the global this in order to support ES Modules.
In this case we have this.DER (which is exported) and class DER in the
same module.

Because of this, changing this.DER to const DER would lead to an error.
So this change renames the class to avoid the conflict.

We need to stop relying on the global this in order to support ES Modules.
This instance was harder to fix by a script than others since it's in a try/catch,
so this patch is a manual change.

https://hg.mozilla.org/mozilla-central/rev/e9ce34f4166a
https://hg.mozilla.org/mozilla-central/rev/809fd7bbf052
https://hg.mozilla.org/mozilla-central/rev/771d32d2e850

https://hg.mozilla.org/mozilla-central/rev/79514ae28ef4