Bug 1240494 - AMD support for DevTools loaders. r=jwalker,Honza draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Tue, 16 Feb 2016 19:50:12 -0600
changeset 331408 11d5c44916e7c95ea74e53f7c304b4458ca995c0
parent 331360 495bca66e5bb8aee6e03584d852f90b4214dc8b0
child 331409 751690106ee39b057982a3ad48218a235d44efef
push id10980
push userbmo:jryans@gmail.com
push dateWed, 17 Feb 2016 02:29:10 +0000
reviewersjwalker, Honza
bugs1240494
milestone47.0a1
Bug 1240494 - AMD support for DevTools loaders. r=jwalker,Honza MozReview-Commit-ID: 7IemsaijHNc
devtools/client/shared/browser-loader.js
devtools/shared/Loader.jsm
--- a/devtools/client/shared/browser-loader.js
+++ b/devtools/client/shared/browser-loader.js
@@ -105,21 +105,32 @@ function BrowserLoader(baseURI, window) 
       }
 
       return require(uri);
     },
     globals: {
       // Allow modules to use the window's console to ensure logs appear in a
       // tab toolbox, if one exists, instead of just the browser console.
       console: window.console,
-      // Make sure 'define' function exists. This allows reusing AMD modules.
-      define: function(callback) {
-        callback(this.require, this.exports, this.module);
-        return this.exports;
-      }
+      // Make sure `define` function exists.  This allows defining some modules
+      // in AMD format while retaining CommonJS compatibility through this hook.
+      // JSON Viewer needs modules in AMD format, as it currently uses RequireJS
+      // from a content document and can't access our usual loaders.  So, any
+      // modules shared with the JSON Viewer should include a define wrapper:
+      //
+      //   // Make this available to both AMD and CJS environments
+      //   define(function(require, exports, module) {
+      //     ... code ...
+      //   });
+      //
+      // Bug 1248830 will work out a better plan here for our content module
+      // loading needs, especially as we head towards devtools.html.
+      define(factory) {
+        factory(this.require, this.exports, this.module);
+      },
     }
   };
 
   if(hotReloadEnabled) {
     opts.loadModuleHook = (module, require) => {
       const { uri, exports } = module;
 
       if (exports.prototype &&
--- a/devtools/shared/Loader.jsm
+++ b/devtools/shared/Loader.jsm
@@ -389,16 +389,32 @@ DevToolsLoader.prototype = {
       loader: {
         lazyGetter: this.lazyGetter,
         lazyImporter: this.lazyImporter,
         lazyServiceGetter: this.lazyServiceGetter,
         lazyRequireGetter: this.lazyRequireGetter,
         id: this.id,
         main: this.main
       },
+      // Make sure `define` function exists.  This allows defining some modules
+      // in AMD format while retaining CommonJS compatibility through this hook.
+      // JSON Viewer needs modules in AMD format, as it currently uses RequireJS
+      // from a content document and can't access our usual loaders.  So, any
+      // modules shared with the JSON Viewer should include a define wrapper:
+      //
+      //   // Make this available to both AMD and CJS environments
+      //   define(function(require, exports, module) {
+      //     ... code ...
+      //   });
+      //
+      // Bug 1248830 will work out a better plan here for our content module
+      // loading needs, especially as we head towards devtools.html.
+      define(factory) {
+        factory(this.require, this.exports, this.module);
+      },
     };
     // Lazy define console in order to load Console.jsm only when it is used
     XPCOMUtils.defineLazyGetter(this._provider.globals, "console", () => {
       return Cu.import("resource://gre/modules/Console.jsm", {}).console;
     });
 
     this._provider.load();
     this.require = Loader.Require(this._provider.loader, { id: "devtools" });