Bug 898034 - Fix module.exports. r=gozala
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Fri, 26 Jul 2013 11:59:53 -0400
changeset 140166 7cdac016f94f2cdf37f9fe082d9226136adeebed
parent 140165 b476f31f27629b19021eb2274787e753f73f025b
child 140167 bcc709f93afb905dada3e188a42964e57c746cc5
push id25016
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:25:56 +0000
treeherdermozilla-central@fb48c7d58b8b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgozala
bugs898034
milestone25.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 898034 - Fix module.exports. r=gozala
toolkit/components/workerloader/require.js
toolkit/components/workerloader/tests/Makefile.in
toolkit/components/workerloader/tests/moduleH-module-dot-exports.js
toolkit/components/workerloader/tests/worker_test_loading.js
--- a/toolkit/components/workerloader/require.js
+++ b/toolkit/components/workerloader/require.js
@@ -163,19 +163,19 @@
         id: path,
         uri: uri,
         exports: exports
       };
 
       // Make module available immediately
       // (necessary in case of circular dependencies)
       if (modules.has(path)) {
-        return modules.get(path);
+        return modules.get(path).exports;
       }
-      modules.set(path, exports);
+      modules.set(path, module);
 
 
       // Load source of module, synchronously
       let xhr = new XMLHttpRequest();
       xhr.open("GET", uri, false);
       xhr.responseType = "text";
       xhr.send();
 
@@ -211,16 +211,17 @@
         if (objectURL) {
           // Clean up the object url as soon as possible. It will not be needed.
           URL.revokeObjectURL(objectURL);
         }
         delete require._tmpModules[name];
       }
 
       Object.freeze(module.exports);
+      Object.freeze(module);
       return module.exports;
     };
   })();
 
   /**
    * An object used to hold temporarily the module constructors
    * while they are being loaded.
    *
@@ -230,9 +231,9 @@
   require._tmpModules = Object.create(null);
   Object.freeze(require);
 
   Object.defineProperty(exports, "require", {
     value: require,
     enumerable: true,
     configurable: false
   });
-})(this);
\ No newline at end of file
+})(this);
--- a/toolkit/components/workerloader/tests/Makefile.in
+++ b/toolkit/components/workerloader/tests/Makefile.in
@@ -17,11 +17,12 @@ MOCHITEST_CHROME_FILES := \
   utils_mainthread.js \
   moduleA-depends.js \
   moduleB-dependency.js \
   moduleC-circular.js \
   moduleD-circular.js \
   moduleE-throws-during-require.js \
   moduleF-syntax-error.js \
   moduleG-throws-later.js \
+  moduleH-module-dot-exports.js \
   $(NULL)
 
 include $(topsrcdir)/config/rules.mk
new file mode 100644
--- /dev/null
+++ b/toolkit/components/workerloader/tests/moduleH-module-dot-exports.js
@@ -0,0 +1,12 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+// This should be overwritten by module.exports
+exports.key = "wrong value";
+
+module.exports = {
+  key: "value"
+};
+
+// This should also be overwritten by module.exports
+exports.key = "another wrong value";
--- a/toolkit/components/workerloader/tests/worker_test_loading.js
+++ b/toolkit/components/workerloader/tests/worker_test_loading.js
@@ -1,87 +1,112 @@
 /* Any copyright is dedicated to the Public Domain.
    http://creativecommons.org/publicdomain/zero/1.0/ */
 
+"use strict";
+
 importScripts("utils_worker.js"); // Test suite code
 info("Test suite configured");
 
 importScripts("resource://gre/modules/workers/require.js");
 info("Loader imported");
 
+let PATH = "chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/";
 let tests = [];
 let add_test = function(test) {
   tests.push(test);
 };
 
 add_test(function test_setup() {
   ok(typeof require != "undefined", "Function |require| is defined");
 });
 
 // Test simple loading (moduleA-depends.js requires moduleB-dependency.js)
 add_test(function test_load() {
-  let A = require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleA-depends.js");
+  let A = require(PATH + "moduleA-depends.js");
   ok(true, "Opened module A");
 
   is(A.A, true, "Module A exported value A");
   ok(!("B" in A), "Module A did not export value B");
   is(A.importedFoo, "foo", "Module A re-exported B.foo");
 
   // re-evaluating moduleB-dependency.js would cause an error, but re-requiring it shouldn't
-  let B = require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleB-dependency.js");
+  let B = require(PATH + "moduleB-dependency.js");
   ok(true, "Managed to re-require module B");
   is(B.B, true, "Module B exported value B");
   is(B.foo, "foo", "Module B exported value foo");
 });
 
 // Test simple circular loading (moduleC-circular.js and moduleD-circular.js require each other)
 add_test(function test_circular() {
-  let C = require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleC-circular.js");
+  let C = require(PATH + "moduleC-circular.js");
   ok(true, "Loaded circular modules C and D");
   is(C.copiedFromD.copiedFromC.enteredC, true, "Properties exported by C before requiring D can be seen by D immediately");
 
-  let D = require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleD-circular.js");
+  let D = require(PATH + "moduleD-circular.js");
   is(D.exportedFromC.finishedC, true, "Properties exported by C after requiring D can be seen by D eventually");
 });
 
 // Testing error cases
 add_test(function test_exceptions() {
   let should_throw = function(f) {
     try {
       f();
       return null;
     } catch (ex) {
       return ex;
     }
   };
 
-  let exn = should_throw(() => require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/this module doesn't exist"));
+  let exn = should_throw(() => require(PATH + "this module doesn't exist"));
   ok(!!exn, "Attempting to load a module that doesn't exist raises an error");
 
-  exn = should_throw(() => require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleE-throws-during-require.js"));
+  exn = should_throw(() => require(PATH + "moduleE-throws-during-require.js"));
   ok(!!exn, "Attempting to load a module that throws at toplevel raises an error");
-  is(exn.moduleName, "chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleE-throws-during-require.js",
+  is(exn.moduleName, PATH + "moduleE-throws-during-require.js",
     "moduleName is correct");
   isnot(exn.moduleStack.indexOf("moduleE-throws-during-require.js"), -1,
     "moduleStack contains the name of the module");
   is(exn.lineNumber, 10, "The error comes with the right line number");
 
-  exn = should_throw(() => require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleF-syntaxerror.xml"));
+  exn = should_throw(() => require(PATH + "moduleF-syntaxerror.xml"));
   ok(!!exn, "Attempting to load a non-well formatted module raises an error");
 
-  exn = should_throw(() => require("chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleG-throws-later.js").doThrow());
+  exn = should_throw(() => require(PATH + "moduleG-throws-later.js").doThrow());
   ok(!!exn, "G.doThrow() has raised an error");
   info(exn);
   ok(exn.toString().startsWith("TypeError"), "The exception is a TypeError.");
-  is(exn.moduleName, "chrome://mochitests/content/chrome/toolkit/components/workerloader/tests/moduleG-throws-later.js", "The name of the module is correct");
+  is(exn.moduleName, PATH + "moduleG-throws-later.js", "The name of the module is correct");
   isnot(exn.moduleStack.indexOf("moduleG-throws-later.js"), -1,
     "The name of the right file appears somewhere in the stack");
   is(exn.lineNumber, 11, "The error comes with the right line number");
 });
 
+function get_exn(f) {
+  try {
+    f();
+    return undefined;
+  } catch (ex) {
+    return ex;
+  }
+}
+
+// Test module.exports
+add_test(function test_module_dot_exports() {
+  let H = require(PATH + "moduleH-module-dot-exports.js");
+  is(H.key, "value", "module.exports worked");
+  let H2 = require(PATH + "moduleH-module-dot-exports.js");
+  is(H2.key, "value", "module.exports returned the same key");
+  ok(H2 === H, "module.exports returned the same module the second time");
+  let exn = get_exn(() => H.key = "this should not be accepted");
+  ok(exn instanceof TypeError, "Cannot alter value in module.exports after export");
+  exn = get_exn(() => H.key2 = "this should not be accepted, either");
+  ok(exn instanceof TypeError, "Cannot add value to module.exports after export");
+});
+
 self.onmessage = function(message) {
   for (let test of tests) {
     info("Entering " + test.name);
     try {
       test();
     } catch (ex) {
       ok(false, "Test " + test.name + " failed");
       info(ex);