Merge pull request #168 from rensbaardman/fix-167-global-var-leakage

lib/getImportGlobalsSrc.js

@@ -9,23 +9,33 @@
  */
 function getImportGlobalsSrc(ignore) {
     var key,
-        value,
         src = "",
         globalObj = typeof global === "undefined"? window: global;
 
     ignore = ignore || [];
-    // global itself can't be overridden because it's the only reference to our real global objects
-    ignore.push("global");
-    // ignore 'module', 'exports' and 'require' on the global scope, because otherwise our code would
-    // shadow the module-internal variables
-    // @see https://github.com/jhnns/rewire-webpack/pull/6
-    ignore.push("module", "exports", "require");
+    ignore.push(
+        // global itself can't be overridden because it's the only reference to our real global objects
+        "global",
+        // ignore 'module', 'exports' and 'require' on the global scope, because otherwise our code would
+        // shadow the module-internal variables
+        // @see https://github.com/jhnns/rewire-webpack/pull/6
+        "module", "exports", "require",
+        // strict mode doesn't allow to (re)define 'undefined', 'eval' & 'arguments'
+        "undefined", "eval", "arguments",
+        // 'GLOBAL' and 'root' are deprecated in Node
+        // (assigning them causes a DeprecationWarning)
+        "GLOBAL", "root",
+        // 'NaN' and 'Infinity' are immutable
+        // (doesn't throw an error if you set 'var NaN = ...', but doesn't work either)
+        "NaN", "Infinity",
+    );
 
-    for (key in globalObj) { /* jshint forin: false */
+    const globals = Object.getOwnPropertyNames(globalObj);
+
+    for (key of globals) {
         if (ignore.indexOf(key) !== -1) {
             continue;
         }
-        value = globalObj[key];
 
         // key may be an invalid variable name (e.g. 'a-b')
         try {

test/getImportGlobalsSrc.test.js

@@ -3,6 +3,7 @@ var expect = require("expect.js"),
     getImportGlobalsSrc = require("../lib/getImportGlobalsSrc.js");
 
 describe("getImportGlobalsSrc", function () {
+
     it("should declare all globals with a var", function () {
         var context = {
                 global: global
@@ -28,34 +29,43 @@ describe("getImportGlobalsSrc", function () {
         delete global['__core-js_shared__'];
         delete global['a-b'];
 
-        expectedGlobals = Object.keys(global);
+        const ignoredGlobals = ["module", "exports", "require", "undefined", "eval", "arguments", "GLOBAL", "root", "NaN", "Infinity"];
+
+        const globals = Object.getOwnPropertyNames(global);
+        expectedGlobals = globals.filter((el) => !ignoredGlobals.includes(el));
 
         vm.runInNewContext(src, context);
-        actualGlobals = Object.keys(context);
+        actualGlobals = Object.getOwnPropertyNames(context);
+
         actualGlobals.sort();
         expectedGlobals.sort();
         expect(actualGlobals).to.eql(expectedGlobals);
         expect(actualGlobals.length).to.be.above(1);
     });
+
     it("should ignore the given variables", function () {
         var context = {
                 global: global
             },
             ignore = ["console", "setTimeout"],
             src,
             actualGlobals,
-            expectedGlobals = Object.keys(global);
+            expectedGlobals = Object.getOwnPropertyNames(global);
+
+        const ignoredGlobals = ["module", "exports", "require", "undefined", "eval", "arguments", "GLOBAL", "root", "NaN", "Infinity"];
+        ignore = ignore.concat(ignoredGlobals);
 
         // getImportGlobalsSrc modifies the ignore array, so let's create a copy
         src = getImportGlobalsSrc(ignore.slice(0));
-        expectedGlobals = expectedGlobals.filter(function filterIgnoredVars(value) {
-            return ignore.indexOf(value) === -1;
-        });
+        expectedGlobals = expectedGlobals.filter((el) => !ignore.includes(el));
+
         vm.runInNewContext(src, context);
         actualGlobals = Object.keys(context);
+
         actualGlobals.sort();
         expectedGlobals.sort();
         expect(actualGlobals).to.eql(expectedGlobals);
         expect(actualGlobals.length).to.be.above(1);
     });
+
 });

testLib/sharedTestCases.js

@@ -177,11 +177,7 @@ module.exports = function () {
         expect(someOtherModule.fs).not.to.be(mockedFs);
     });
 
-    // This test fails on modern Node versions since they started to configure some
-    // global variables to be non-enumerable. This means that rewire() does in fact
-    // modify the global console object in newer Node versions.
-    // There is a work in progress fix at https://github.com/jhnns/rewire/tree/fix-globals
-    it.skip("should provide the ability to mock global objects just within the module", function () {
+    it("should provide the ability to mock global objects just within the module", function () {
         var rewiredModuleA = rewire("./moduleA.js"),
             rewiredModuleB = rewire("./moduleB.js"),
             consoleMock = {},