Merge pull request #67 from rwjblue/refactorzzzz

rwjblue · web-flow · commit 32c776da43ec · 2018-05-23T20:57:53.000-04:00
Unify flag handling...
diff --git a/README.md b/README.md
@@ -4,33 +4,43 @@ This provides debug macros and feature flagging.
 
 ## Setup
 
-The plugin takes 5 types options: `envFlags`, `features`, `debugTools`, `externalizeHelpers` and `svelte`. The `importSpecifier` is used as a hint to this plugin as to where macros are being imported and completely configurable by the host. Like Babel you can supply your own helpers using the `externalizeHelpers` options.
+The plugin takes 4 types options: `flags`, `svelte`, `debugTools`, and
+`externalizeHelpers`. The `importSpecifier` is used as a hint to this plugin as
+to where macros are being imported and completely configurable by the host.
+
+Like Babel you can supply your own helpers using the `externalizeHelpers`
+options.
 
 ```js
 {
   plugins: [
     ['babel-debug-macros', {
-      // @required
-      envFlags: {
-        source: '@ember/env-flags',
-        flags: { DEBUG: true }
-      },
       // @required
       debugTools: {
+        isDebug: true,
         source: 'debug-tools',
         // @optional
         assertPredicateIndex: 0
       },
-      // @optional
-      features: {
-        name: 'ember-source',
-        source: '@ember/features',
-        flags: { FEATURE_A: false, FEATURE_B: true, DEPRECATED_CONTROLLERS: "2.12.0" }
-      },
+
+      flags: [
+        { source: '@ember/env-flags', flags: { DEBUG: true } },
+        {
+          name: 'ember-source',
+          source: '@ember/features',
+          flags: {
+            FEATURE_A: false,
+            FEATURE_B: true,
+            DEPRECATED_CONTROLLERS: "2.12.0"
+          }
+        }
+      ],
+
       // @optional
       svelte: {
         'ember-source': "2.15.0"
       },
+
       // @optional
       externalizeHelpers: {
         module: true,
@@ -66,12 +76,12 @@ woot();
 Transforms to:
 
 ```javascript
-if (true) {
+if (true /* DEBUG */) {
   console.log('Hello from debug');
 }
 
 let woot;
-if (false) {
+if (false /* FEATURE_A */) {
   woot = () => 'woot';
 } else if (true) {
   woot = () => 'toow';
@@ -141,7 +151,9 @@ let foo = 2;
 
 ## Externalized Helpers
 
-When you externalize helpers you must provide runtime implementations for the above macros. An expansion will still occur, however we will emit references to those runtime helpers.
+When you externalize helpers you must provide runtime implementations for the
+above macros. An expansion will still occur, however we will emit references to
+those runtime helpers.
 
 A global expansion looks like the following:
 
@@ -173,26 +185,28 @@ Expands into:
 
 # Svelte
 
-Svelte allows for consumers to opt into stripping deprecated code from your dependecies. By adding a package name and minimum version that contains no deprecations, that code will be compiled away.
+Svelte allows for consumers to opt into stripping deprecated code from your
+dependecies. By adding a package name and minimum version that contains no
+deprecations, that code will be compiled away.
 
-For example, consider you are on `ember-source@2.10.0` and you have no deprecations. All deprecated code in `ember-source` that is `<=2.10.0` will be removed.
+For example, consider you are on `ember-source@2.10.0` and you have no
+deprecations. All deprecated code in `ember-source` that is `<=2.10.0` will be
+removed.
 
 ```
-...
+
 svelte: {
   "ember-source": "2.10.0"
 }
-...
+
 ```
 
-Now if you bump to `ember-source@2.11.0` you may encounter new deprecations. The workflow would then be to clear out all deprecations and then bump the version in the `svelte` options.
+Now if you bump to `ember-source@2.11.0` you may encounter new deprecations.
+The workflow would then be to clear out all deprecations and then bump the
+version in the `svelte` options.
 
 ```
 svelte: {
   "ember-source": "2.11.0"
 }
 ```
-
-# Hygenic
-
-As you may notice that we inject `DEBUG` into the code when we expand the macro. We guarantee that the binding is unique when injected and follow the local binding name if it is imported directly.
diff --git a/fixtures/debug-flag/expectation6.js b/fixtures/debug-flag/expectation6.js
@@ -1,5 +1,5 @@
 
 
-if (true) {
+if (true /* DEBUG */) {
   console.log('woot');
 }
diff --git a/fixtures/debug-flag/expectation7.js b/fixtures/debug-flag/expectation7.js
@@ -1,3 +1,5 @@
-if (true) {
+if (true
+/* DEBUG */
+) {
   console.log('woot');
 }
diff --git a/fixtures/ember-cli-babel-config/expectation6.js b/fixtures/ember-cli-babel-config/expectation6.js
@@ -1,6 +1,6 @@
 
 
-if (true) {
+if (true /* DEBUG */) {
   doStuff();
 }
 
diff --git a/fixtures/ember-cli-babel-config/expectation7.js b/fixtures/ember-cli-babel-config/expectation7.js
@@ -1,4 +1,6 @@
-if (true) {
+if (true
+/* DEBUG */
+) {
   doStuff();
 }
 
diff --git a/fixtures/inject-env-flags/expectation6.js b/fixtures/inject-env-flags/expectation6.js
@@ -1,11 +1,11 @@
 
 
 let testing;
-if (false) {
+if (false /* TESTING */) {
   testing = 'WOOT';
 }
 
 let debug;
-if (true) {
+if (true /* DEBUG */) {
   debug = 'DEBUG';
 }
diff --git a/fixtures/inject-env-flags/expectation7.js b/fixtures/inject-env-flags/expectation7.js
@@ -1,11 +1,15 @@
 let testing;
 
-if (false) {
+if (false
+/* TESTING */
+) {
   testing = 'WOOT';
 }
 
 let debug;
 
-if (true) {
+if (true
+/* DEBUG */
+) {
   debug = 'DEBUG';
 }
diff --git a/fixtures/inline-feature-flags/expectation6.js b/fixtures/inline-feature-flags/expectation6.js
@@ -1,18 +1,18 @@
 
 
 let a;
-if (false) {
+if (false /* FEATURE_A */) {
   a = () => console.log('hello');
-} else if (true) {
+} else if (true /* FEATURE_B */) {
   a = () => console.log('bye');
 }
 
-if (!false) {
+if (!false /* FEATURE_A */) {
   console.log('stuff');
 }
 
-a = false ? 'hello' : 'bye';
+a = false /* FEATURE_A */ ? 'hello' : 'bye';
 
-if (false && window.foo && window.bar) {
+if (false /* FEATURE_A */ && window.foo && window.bar) {
   console.log('wheeee');
 }
diff --git a/fixtures/inline-feature-flags/expectation7.js b/fixtures/inline-feature-flags/expectation7.js
@@ -1,17 +1,27 @@
 let a;
 
-if (false) {
+if (false
+/* FEATURE_A */
+) {
   a = () => console.log('hello');
-} else if (true) {
+} else if (true
+/* FEATURE_B */
+) {
   a = () => console.log('bye');
 }
 
-if (!false) {
+if (!false
+/* FEATURE_A */
+) {
   console.log('stuff');
 }
 
-a = false ? 'hello' : 'bye';
+a = false
+/* FEATURE_A */
+? 'hello' : 'bye';
 
-if (false && window.foo && window.bar) {
+if (false
+/* FEATURE_A */
+&& window.foo && window.bar) {
   console.log('wheeee');
 }
diff --git a/fixtures/runtime-feature-flags/expectation6.js b/fixtures/runtime-feature-flags/expectation6.js
@@ -1,6 +1,6 @@
 import { FEATURE_B } from '@ember/features';
 
-if (true) {
+if (true /* FEATURE_A */) {
   console.log('woot');
 }
 
diff --git a/fixtures/runtime-feature-flags/expectation7.js b/fixtures/runtime-feature-flags/expectation7.js
@@ -1,6 +1,8 @@
 import { FEATURE_B } from '@ember/features';
 
-if (true) {
+if (true
+/* FEATURE_A */
+) {
   console.log('woot');
 }
 
diff --git a/src/index.js b/src/index.js
@@ -5,11 +5,70 @@ const Macros = require('./utils/macros');
 const normalizeOptions = require('./utils/normalize-options').normalizeOptions;
 
 function macros(babel) {
+  let t = babel.types;
   let options;
 
+  function buildIdentifier(value, name) {
+    let replacement = t.booleanLiteral(value);
+
+    // when we only support babel@7 we should change this
+    // to `path.addComment` or `t.addComment`
+    let comment = {
+      type: 'CommentBlock',
+      value: ` ${name} `,
+      leading: false,
+      trailing: true,
+    };
+    replacement.trailingComments = [comment];
+
+    return replacement;
+  }
+
   return {
     name: 'babel-feature-flags-and-debug-macros',
     visitor: {
+      ImportSpecifier(path) {
+        let importPath = path.parent.source.value;
+        let flagsForImport = options.flags[importPath];
+
+        if (flagsForImport) {
+          let flagName = path.node.imported.name;
+          let localBindingName = path.node.local.name;
+
+          if (!(flagName in flagsForImport)) {
+            throw new Error(
+              `Imported ${flagName} from ${importPath} which is not a supported flag.`
+            );
+          }
+
+          let flagValue = flagsForImport[flagName];
+          if (flagValue === null) {
+            return;
+          }
+
+          let binding = path.scope.getBinding(localBindingName);
+
+          binding.referencePaths.forEach(p => {
+            p.replaceWith(buildIdentifier(flagValue, flagName));
+          });
+
+          path.remove();
+          path.scope.removeOwnBinding(localBindingName);
+        }
+      },
+
+      ImportDeclaration: {
+        exit(path) {
+          let importPath = path.node.source.value;
+          let flagsForImport = options.flags[importPath];
+
+          // remove flag source imports when no specifiers are left
+          if (flagsForImport && path.get('specifiers').length === 0) {
+            path.remove();
+          }
+        },
+      },
+
       Program: {
         enter(path, state) {
           options = normalizeOptions(state.opts);
@@ -22,14 +81,10 @@ function macros(babel) {
               let importPath = item.node.source.value;
 
               let debugToolsImport = options.debugTools.debugToolsImport;
-              let envFlagsImport = options.envFlags.envFlagsImport;
 
               if (debugToolsImport && debugToolsImport === importPath) {
                 this.macroBuilder.collectDebugToolsSpecifiers(item.get('specifiers'));
               }
-              if (envFlagsImport && envFlagsImport === importPath) {
-                this.macroBuilder.collectEnvFlagSpecifiers(item.get('specifiers'));
-              }
             }
           });
         },
diff --git a/src/utils/builder.js b/src/utils/builder.js
@@ -6,6 +6,7 @@ module.exports = class Builder {
     this.module = options.module;
     this.global = options.global;
     this.assertPredicateIndex = options.assertPredicateIndex;
+    this.isDebug = options.isDebug;
     this.expressions = [];
   }
 
@@ -170,9 +171,9 @@ module.exports = class Builder {
   /**
    * Performs the actually expansion of macros
    */
-  expandMacros(debugFlag) {
+  expandMacros() {
     let t = this.t;
-    let flag = t.booleanLiteral(debugFlag);
+    let flag = t.booleanLiteral(this.isDebug);
     for (let i = 0; i < this.expressions.length; i++) {
       let expression = this.expressions[i];
       let exp = expression[0];
diff --git a/src/utils/macros.js b/src/utils/macros.js
diff --git a/src/utils/normalize-options.js b/src/utils/normalize-options.js
diff --git a/tests/create-tests.js b/tests/create-tests.js
diff --git a/tests/normalize-options-test.js b/tests/normalize-options-test.js

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`
`2`	`2`
`3`		`-if (true) {`
	`3`	`+if (true /* DEBUG */) {`
`4`	`4`	`console.log('woot');`
`5`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
`1`		`-if (true) {`
	`1`	`+if (true`
	`2`	`+/* DEBUG */`
	`3`	`+) {`
`2`	`4`	`console.log('woot');`
`3`	`5`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`
`2`	`2`
`3`		`-if (true) {`
	`3`	`+if (true /* DEBUG */) {`
`4`	`4`	`doStuff();`
`5`	`5`	`}`
`6`	`6`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,6 @@`
`1`		`-if (true) {`
	`1`	`+if (true`
	`2`	`+/* DEBUG */`
	`3`	`+) {`
`2`	`4`	`doStuff();`
`3`	`5`	`}`
`4`	`6`
Original file line number	Diff line number	Diff line change
`@@ -1,11 +1,11 @@`
`1`	`1`
`2`	`2`
`3`	`3`	`let testing;`
`4`		`-if (false) {`
	`4`	`+if (false /* TESTING */) {`
`5`	`5`	`testing = 'WOOT';`
`6`	`6`	`}`
`7`	`7`
`8`	`8`	`let debug;`
`9`		`-if (true) {`
	`9`	`+if (true /* DEBUG */) {`
`10`	`10`	`debug = 'DEBUG';`
`11`	`11`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,18 +1,18 @@`
`1`	`1`
`2`	`2`
`3`	`3`	`let a;`
`4`		`-if (false) {`
	`4`	`+if (false /* FEATURE_A */) {`
`5`	`5`	`a = () => console.log('hello');`
`6`		`-} else if (true) {`
	`6`	`+} else if (true /* FEATURE_B */) {`
`7`	`7`	`a = () => console.log('bye');`
`8`	`8`	`}`
`9`	`9`
`10`		`-if (!false) {`
	`10`	`+if (!false /* FEATURE_A */) {`
`11`	`11`	`console.log('stuff');`
`12`	`12`	`}`
`13`	`13`
`14`		`-a = false ? 'hello' : 'bye';`
	`14`	`+a = false /* FEATURE_A */ ? 'hello' : 'bye';`
`15`	`15`
`16`		`-if (false && window.foo && window.bar) {`
	`16`	`+if (false /* FEATURE_A */ && window.foo && window.bar) {`
`17`	`17`	`console.log('wheeee');`
`18`	`18`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,6 +1,6 @@`
`1`	`1`	`import { FEATURE_B } from '@ember/features';`
`2`	`2`
`3`		`-if (true) {`
	`3`	`+if (true /* FEATURE_A */) {`
`4`	`4`	`console.log('woot');`
`5`	`5`	`}`
`6`	`6`