This might be a lot of work but we need a bug for it. In order to reload our tools, we need to avoid using `Cu.import` anywhere, which involves not using .jsm files. I don't see any major barriers for doing this, just a lot of grunt work to clean up our files.

A couple of random thoughts: How far do we go with this e.g. does this mean moving Services.jsm to a js file and then using require? It would also be great to always use lazy getters but then we couldn't deconstruct values: e.g. let {x, y, z} = Cu.Import("path/blah.jsm");

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #1) > A couple of random thoughts: > How far do we go with this e.g. does this mean moving Services.jsm to a js > file and then using require? I doubt it, for special cases like that every module just needs to make sure that if they mutate a service in any way, they "unmutate" it when being reloaded. For example adding an event handler on the Observer service. I already ran into this when playing with loaders: promise and event-emitter don't unregister themselves from the observer service, but it shouldn't be hard to do that. These services are global C++ singleton services (as far as I know) so we'll always have to deal with them like that. > It would also be great to always use lazy getters but then we couldn't > deconstruct values: > e.g. let {x, y, z} = Cu.Import("path/blah.jsm"); Yeah, I'm not convinced that we really need to lazy load that much. We shouldn't ever need to lazy load shared files, as they are almost always needed immediately anyway. The only times we should lazy load in my opinion is when a tool tried to access a different tool (like the console loads a location in the debugger), the console shouldn't load the whole debugger.

Thinking about it more, I guess there are several places where it's important to lazy-load, but Jordan is working through the loader APIs to make that easy, so we'll figure that out.

Note that JSMs can be unloaded too, but we would still prefer a more common API (especially outside Mozilla) like require(): https://developer.mozilla.org/en-US/docs/Mozilla/Tech/XPCOM/Language_Bindings/Components.utils.unload

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8893f500dc03 I kept that out of the refactoring patch, as we may want to keep it for addons?? But I ran on try with this patch to see if that pass without it!

Comment on attachment 8639766 [details] [diff] [review] Use DevToolsUtils module instead of JSM This patch is really naive. Replace Cu.import(DevToolsUtils.jsm) by require(DevToolsUtils.js) with some related cleanup here and there. Note that there is no need to lazy load some modules if: - they are already loaded (Loader.jsm) - they are going to be used by the related module no matter what (DevtoolsUtils in some cases)

Comment on attachment 8639767 [details] [diff] [review] Remove DevToolsUtils.jsm This one is more at risk as it's not clear if any addon is using this module or not.

(In reply to Alexandre Poirot [:ochameau] from comment #8) > Comment on attachment 8639767 [details] [diff] [review] > Remove DevToolsUtils.jsm > > This one is more at risk as it's not clear if any addon is using this module > or not. If I look at this, there is only two addons using it on AMO: https://mxr.mozilla.org/addons/search?string=DevToolsUtils.jsm&find=&findi=&filter=^[^\0]*%24&hitlimit=&tree=addons ICS Inspector Remote Developer Tools Server Both written by Philipp Kewisch

Comment on attachment 8639766 [details] [diff] [review] Use DevToolsUtils module instead of JSM Review of attachment 8639766 [details] [diff] [review]: ----------------------------------------------------------------- My opinion is that we should avoid devtools.require and define a require const instead to make it easier to move to an html5 module system in the future. I wouldn't r- this for that though.

https://hg.mozilla.org/integration/fx-team/rev/5f81d6f390c5 https://hg.mozilla.org/integration/fx-team/rev/825a6e29bff1

Philipp, this is just to let you know that your two addons may break with upcoming Fx release. You would just need to replace: Cu.import("resource://gre/modules/devtools/DevToolsUtils.jsm"); By: let {devtools} = Cu.import("resource://gre/modules/devtools/Loader.jsm", {}); let DevToolsUtils = devtools.require("devtools/toolkit/DevToolsUtils");

(In reply to Michael Ratcliffe [:miker] [:mratcliffe] from comment #1) > A couple of random thoughts: > How far do we go with this e.g. does this mean moving Services.jsm to a js > file and then using require? My initiative here is to convert most *devtools* JSM, that to ensure that `tools srcdir`/`tools reload` gcli commands, correctly reload devtools from sources. We plan to eventually have devtools sources in a distinct repo. So that we only care about devtools modules, the others are going to be pulled out of Firefox resources.

(In reply to Joe Walker [:jwalker] (overloaded - needinfo me or ping on irc) from comment #10) > Comment on attachment 8639766 [details] [diff] [review] > Use DevToolsUtils module instead of JSM > > Review of attachment 8639766 [details] [diff] [review]: > ----------------------------------------------------------------- > > My opinion is that we should avoid devtools.require and define a require > const instead to make it easier to move to an html5 module system in the > future. Sounds like a separate issue. I do have idea that would ease that and could be another refactoring. See bug 1172010 comment 2. In most cases I just reuse devtools.require, like other usage in the same file. I filled bug 1188401.

Similar move but for LayoutHelpers.jsm. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8c874cde522

Thanks for working on this! re-assigning to you for now. I was going to audit our usage of it soon but it's great your actually landing changes.

Thanks for the note. I'll make sure to change this. From what I have been reading I believe the new way to import is backwards compatible, in case not please let me know.

https://hg.mozilla.org/mozilla-central/rev/5f81d6f390c5 https://hg.mozilla.org/mozilla-central/rev/825a6e29bff1

This time, with a green try. https://treeherder.mozilla.org/#/jobs?repo=try&revision=e8c874cde522 I removed the JSM in this patch as it sounds even less used than DevToolsUtils. But actually, two addons are using it: https://mxr.mozilla.org/addons/search?string=LayoutHelpers.jsm DevTools Tweaks by Luke Bryan (Pinged here about this - https://github.com/programmin1/DevTools-Tweaks/issues/6) YouTube Animated Thumbnails by victor (Will ni? him)

Victor, please ack the upcoming breakage for your "YouTube Animated Thumbnails" addon.

I'm going to handle SourceMap.jsm in existing bug 935366.

(In reply to Alexandre Poirot [:ochameau] from comment #20) > Victor, please ack the upcoming breakage for your "YouTube Animated > Thumbnails" addon. Ugh, ok.