1530868 - Fix module imports in chat code

Status: RESOLVED FIXED

(Chat Core :: General, enhancement)

Description

[Patrick Cloke [:clokep]]

Bug 1520643 made a bunch of changes to how we import modules, unfortunately as part of this it seems that all of the possible exported symbols for each module were imported into each other file inside of chat. Oddly there's also a few spots where we're missing the proper imports.

Comment 1

[Patrick Cloke [:clokep]]

Attachment: Patch v1

The locations to fix were found by using eslint and looking at the no-undef and no-unused-vars errors.

Note that it seems there were some place where modules were imported that were never used at all!

Comment 2

[Martin Giger [:freaktechnik]]

Comment on attachment 9046885 [details] [diff] [review]
Patch v1

Review of attachment 9046885 [details] [diff] [review]:
-----------------------------------------------------------------

I'm assuming the additions/removals work, given that I don't currently have a build setup. They all look sensible and assuming all the paths acutall work (I couldn't find any obvious typos) and eslint doesn't complain this should be fine.

::: chat/protocols/irc/ircCTCP.jsm
@@ +14,2 @@
>  const {ircHandlers} = ChromeUtils.import("resource:///modules/ircHandlers.jsm");
> +var {_} = ChromeUtils.import("resource:///modules/ircUtils.jsm");

It's a bit weird to have one import that uses var and the other two using const, though var seems to be the more prevalent one? I think the different styles come from multiple code transformations and when the code was written. Is the chosen style to always use var for new ones, and existing const ones are not touched?

::: chat/protocols/matrix/matrix-sdk.jsm
@@ -7,2 @@
>  const {clearInterval, clearTimeout, setInterval, setTimeout} = ChromeUtils.import("resource://gre/modules/Timer.jsm");
> -Cu.importGlobalProperties(["XMLHttpRequest"]);

Huh, is XHR now just available in JSMs?

Comment 3

[Patrick Cloke [:clokep]]

(In reply to Martin Giger [:freaktechnik] from comment #2)

::: chat/protocols/irc/ircCTCP.jsm
@@ +14,2 @@

const {ircHandlers} = ChromeUtils.import("resource:///modules/ircHandlers.jsm");
+var {_} = ChromeUtils.import("resource:///modules/ircUtils.jsm");

It's a bit weird to have one import that uses var and the other two using
const, though var seems to be the more prevalent one? I think the different
styles come from multiple code transformations and when the code was
written. Is the chosen style to always use var for new ones, and existing
const ones are not touched?

I also find this super weird that it is sometimes var and sometimes const. florian and I discussed this on IRC and it seems they should each be const, but it doesn't really matter. I'm going to ignore this for now.

::: chat/protocols/matrix/matrix-sdk.jsm
@@ -7,2 @@

const {clearInterval, clearTimeout, setInterval, setTimeout} = ChromeUtils.import("resource://gre/modules/Timer.jsm");
-Cu.importGlobalProperties(["XMLHttpRequest"]);

Huh, is XHR now just available in JSMs?

That's what eslint told me. This code needs to be re-worked anyway, so I don't think this is a big deal.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/5b2feb6ef349
Fix imports in chat after bug 1520643. r=freaktechnik

Comment 5

[Magnus Melin [:mkmelin]]

Re var vs const: yes in modules the preferred way is really const.
We had to change a bunch of non-module js files to use var since otherwise there would be failures due to re-assignment...