Bug 898034 - Fix module.exports. r=gozala
authorDavid Rajchenbach-Teller <dteller@mozilla.com>
Fri, 26 Jul 2013 11:59:53 -0400
changeset 140173 7cdac016f94f2cdf37f9fe082d9226136adeebed
parent 140172 b476f31f27629b19021eb2274787e753f73f025b
child 140174 bcc709f93afb905dada3e188a42964e57c746cc5
push id1945
push userryanvm@gmail.com
push dateSat, 27 Jul 2013 02:27:26 +0000
treeherderfx-team@4874fa438b1c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgozala
bugs898034
milestone25.0a1
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);