Merge branch 'master' into pr/origin/127

2 files changed, 57 insertions, 14 deletions

diff --git a/lib/moduleEnv.js b/lib/moduleEnv.js
index 2f9743c..599e332 100644
--- a/lib/moduleEnv.js
+++ b/lib/moduleEnv.js
@@ -1,16 +1,48 @@
 "use strict";
 
+// TODO: Use https://www.npmjs.com/package/pirates here?
+
 var Module = require("module"),
     fs = require("fs"),
-    babelCore = require("babel-core"),
-    // Requiring the babel plugin here because otherwise it will be lazy-loaded by Babel during rewire()
-    transformBlockScoping = require("babel-plugin-transform-es2015-block-scoping"),
+    eslint = require("eslint"),
     coffee;
 
 var moduleWrapper0 = Module.wrapper[0],
     moduleWrapper1 = Module.wrapper[1],
     originalExtensions = {},
-    matchCoffeeExt = /\.coffee$/,
+    linter = new eslint.Linter(),
+    eslintOptions = {
+        env: {
+            es6: true,
+        },
+        parserOptions: {
+            ecmaVersion: 6,
+            ecmaFeatures: {
+                globalReturn: true,
+                jsx: true,
+                experimentalObjectRestSpread: true
+            },
+        },
+        rules: {
+            "no-const-assign": 2
+        }
+    },
+    // The following regular expression is used to replace const declarations with let.
+    // This regex replacement is not 100% safe because transforming JavaScript requires an actual parser.
+    // However, parsing (e.g. via babel) comes with its own problems because now the parser needs to
+    // be aware of syntax extensions which might not be supported by the parser, but the underlying
+    // JavaScript engine. In fact, rewire used to have babel in place here but required an extra
+    // transform for the object spread operator (check out commit d9a81c0cdacf6995b24d205b4a2068adbd8b34ff
+    // or see https://github.com/jhnns/rewire/pull/128). It was also notable slower
+    // (see https://github.com/jhnns/rewire/issues/132).
+    // There is another issue: replacing const with let is not safe because of their different behavior.
+    // That's why we also have ESLint in place which tries to identify this error case.
+    // There is one edge case though: when a new syntax is used *and* a const re-assignment happens,
+    // rewire would compile happily in this situation but the actual code wouldn't work.
+    // However, since most projects have a seperate linting step which catches these const re-assignment
+    // errors anyway, it's probably still a reasonable trade-off.
+    // Test the regular expresssion at https://regex101.com/r/dvnZPv/2 and also check out testLib/constModule.js.
+    matchConst = /(^|\s|\}|;)const(\/\*|\s)/gm,
     nodeRequire,
     currentModule;
 
@@ -71,17 +103,29 @@ function restoreExtensions() {
     }
 }
 
+function isNoConstAssignMessage(message) {
+    return message.ruleId === "no-const-assign";
+}
+
 function jsExtension(module, filename) {
     var _compile = module._compile;
 
     module._compile = function (content, filename) {
-        content = babelCore.transform(content, {
-            plugins: [require.resolve("babel-plugin-transform-es2015-block-scoping")],
-            retainLines: true,
-            filename: filename,
-            babelrc: false
-        }).code;
-        _compile.call(module, content, filename);
+        var noConstAssignMessage = linter.verify(content, eslintOptions).find(isNoConstAssignMessage);
+        var line;
+        var column;
+
+        if (noConstAssignMessage !== undefined) {
+            line = noConstAssignMessage.line;
+            column = noConstAssignMessage.column;
+            throw new TypeError(`Assignment to constant variable at ${ filename }:${ line }:${ column }`);
+        }
+
+        _compile.call(
+            module,
+            content.replace(matchConst, "$1let  $2"), // replace const with let, while maintaining the column width
+            filename
+        );
     };
 
     restoreExtensions();
@@ -90,7 +134,7 @@ function jsExtension(module, filename) {
 
 function coffeeExtension(module, filename) {
     if (!coffee) {
-        throw new Error("please add 'coffeescript' to your devDependencies");
+        throw new Error("Cannot rewire module written in CoffeeScript: Please install 'coffeescript' package first.");
     }
 
     var content = stripBOM(fs.readFileSync(filename, "utf8"));
@@ -123,7 +167,7 @@ try {
         // Trying to load deprecated package
         coffee = require("coffee-script");
     } catch (err) {
-        // We are not able to provide coffee-script support, but that's ok as long as the user doesn't want it.
+        // We are not able to provide CoffeeScript support, but that's ok as long as the user doesn't want it.
     }
 }
 
diff --git a/lib/rewire.js b/lib/rewire.js
index e678f37..3ac2042 100644
--- a/lib/rewire.js
+++ b/lib/rewire.js
@@ -1,6 +1,5 @@
 var Module = require("module"),
     fs = require("fs"),
-    path = require("path"),
     getImportGlobalsSrc = require("./getImportGlobalsSrc.js"),
     getDefinePropertySrc = require("./getDefinePropertySrc.js"),
     detectStrictMode = require("./detectStrictMode.js"),