Merge pull request #24 from zalando-incubator/fix-globals

Fix locals got shadowed by globals 🤦‍♂️
peter-leonov · May 31, 2019 · 1315ce0 · 1315ce0
2 parents 19e0ab9 + 25b5720
commit 1315ce0
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 19 deletions.
diff --git a/babel-plugin.js b/babel-plugin.js
@@ -1,3 +1,6 @@
+// think of rewriting all this mess with just reusing rewriteModuleStatementsAndPrepareHeader from:
+// https://github.com/babel/babel/blob/master/packages/babel-plugin-transform-modules-commonjs/src/index.js#L147
+
 const toPairs = obj => {
     const pairs = [];
     for (const key in obj) {
@@ -435,10 +438,13 @@ function processProgram({ types: t }, programPath, programOpts) {
         );
 
         // globals become locals after wrapInFunction()
-        // TODO: make it more explicit in the code
+        let once = false;
         const globalBindings = {};
         path.traverse({
             Scope(path, state) {
+                // process scope of only the introscope() function
+                if (once) return;
+                once = true;
                 programGlobalNames.forEach(globalName => {
                     const binding = path.scope.getBinding(globalName);
                     globalBindings[globalName] = binding;

diff --git a/test/__snapshots__/plugin.spec.js.snap b/test/__snapshots__/plugin.spec.js.snap
@@ -437,7 +437,29 @@ export const introscope = function (_scope = {}) {
 };"
 `;
 
-exports[`plugin globals 1`] = `
+exports[`plugin globals does not break locals with same name as globals 1`] = `
+"var _savedGlobal = global;
+export const introscope = function (_scope = {}) {
+  var someGlobal = _scope.someGlobal = _savedGlobal.someGlobal;
+  _scope.aFunctionWaskingGlobal = aFunctionWaskingGlobal;
+
+  _scope.someGlobal.x1();
+
+  function aFunctionWaskingGlobal() {
+    let someGlobal = 123;
+    someGlobal.y();
+  }
+
+  ;
+
+  _scope.someGlobal.x2();
+
+  _scope.someGlobal = 123;
+  return _scope;
+};"
+`;
+
+exports[`plugin globals supports global functions 1`] = `
 "var _savedGlobal = global;
 export const introscope = function (_scope = {}) {
   var global2 = _scope.global2 = _savedGlobal.global2,
@@ -455,7 +477,7 @@ export const introscope = function (_scope = {}) {
 };"
 `;
 
-exports[`plugin globals 2`] = `
+exports[`plugin globals supports ignore for globals 1`] = `
 "var _savedGlobal = global;
 export const introscope = function (_scope = {}) {
   var global = _scope.global = _savedGlobal.global;
@@ -471,7 +493,7 @@ export const introscope = function (_scope = {}) {
 };"
 `;
 
-exports[`plugin globals 3`] = `
+exports[`plugin globals supports nested globals 1`] = `
 "var _savedGlobal = global;
 export const introscope = function (_scope = {}) {
   _scope.getGlobal = getGlobal;

diff --git a/test/plugin.spec.js b/test/plugin.spec.js
@@ -110,23 +110,41 @@ describe('plugin', () => {
         `);
     });
 
-    it('globals', () => {
-        shoot(`
-            let x = global1;
-            global2 = 1;
-            global2 = 1;
-            globalFunction();
-            !function(){ return [nestedGlobal1, global2] }();
-        `);
+    describe('globals', () => {
+        it('supports global functions', () => {
+            shoot(`
+                let x = global1;
+                global2 = 1;
+                global2 = 1;
+                globalFunction();
+                !function(){ return [nestedGlobal1, global2] }();
+            `);
+        });
 
-        shoot(`
-            function getGlobal () { return global };
-            // @introscope "ignore": ["-global"]
-        `);
+        it('supports ignore for globals', () => {
+            shoot(`
+                function getGlobal () { return global };
+                // @introscope "ignore": ["-global"]
+            `);
+        });
 
-        shoot(`
-            function getGlobal () { return global };
-        `);
+        it('supports nested globals', () => {
+            shoot(`
+                function getGlobal () { return global };
+            `);
+        });
+
+        it('does not break locals with same name as globals', () => {
+            shoot(`
+            someGlobal.x1()
+            function aFunctionWaskingGlobal () {
+                let someGlobal = 123
+                someGlobal.y()
+            };
+            someGlobal.x2()
+            someGlobal = 123
+            `);
+        });
     });
 
     it('export', () => {