fix: order of application in non-macro transforms chain (#96)

* Order of application in non-macro transforms chain * plugins order test and coverage fix for #96
kentcdodds · Dec 17, 2018 · 2e0badb · 2e0badb
1 parent 9cc8af1
commit 2e0badb
Show file tree

Hide file tree

Showing 5 changed files with 133 additions and 63 deletions.
diff --git a/src/__tests__/__snapshots__/index.js.snap b/src/__tests__/__snapshots__/index.js.snap
@@ -1,5 +1,17 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`macros Macros are applied in the order respecting plugins order: Macros are applied in the order respecting plugins order 1`] = `
+
+import Wrap from "./fixtures/jsx-id-prefix.macro";
+
+const bar = Wrap(<div id="d1"><p id="p1"></p></div>);
+
+      ↓ ↓ ↓ ↓ ↓ ↓
+
+const bar = Wrap(<div id="plugin-macro-d1"><p id="plugin-macro-p1"></p></div>);
+
+`;
+
 exports[`macros Supports named imports: Supports named imports 1`] = `
 
 import {css as CSS, styled as STYLED} from './fixtures/emotion.macro'

diff --git a/src/__tests__/fixtures/jsx-id-prefix.macro.js b/src/__tests__/fixtures/jsx-id-prefix.macro.js
@@ -0,0 +1,20 @@
+// adds "prefix-" to each `id` attribute
+const {createMacro} = require('../../')
+
+module.exports = createMacro(wrapWidget)
+
+function wrapWidget({references, babel}) {
+  const {types: t} = babel
+  references.default.forEach(wrap => {
+    wrap.parentPath.traverse({
+      JSXAttribute(path) {
+        const name = path.get('name')
+        if (t.isJSXIdentifier(name) && name.node.name === 'id') {
+          const value = path.get('value')
+          if (t.isStringLiteral(value))
+            value.replaceWith(t.stringLiteral(`macro-${value.node.value}`))
+        }
+      },
+    })
+  })
+}
diff --git a/src/__tests__/fixtures/jsx-id-prefix.plugin.js b/src/__tests__/fixtures/jsx-id-prefix.plugin.js
@@ -0,0 +1,23 @@
+// babel-plugin adding `plugin-` prefix to each "id" JSX attribute
+module.exports = main
+
+function main({types: t}) {
+  return {
+    visitor: {
+      // intentionally traversing from Program,
+      // if it matches JSXAttribute here the issue won't be reproduced
+      Program(progPath) {
+        progPath.traverse({
+          JSXAttribute(path) {
+            const name = path.get('name')
+            if (t.isJSXIdentifier(name) && name.node.name === 'id') {
+              const value = path.get('value')
+              if (t.isStringLiteral(value))
+                value.replaceWith(t.stringLiteral(`plugin-${value.node.value}`))
+            }
+          },
+        })
+      },
+    },
+  }
+}
diff --git a/src/__tests__/index.js b/src/__tests__/index.js
@@ -315,5 +315,16 @@ pluginTester({
         babelrc: true,
       },
     },
+    {
+      title: 'Macros are applied in the order respecting plugins order',
+      code: `
+        import Wrap from "./fixtures/jsx-id-prefix.macro";
+
+        const bar = Wrap(<div id="d1"><p id="p1"></p></div>);
+      `,
+      babelOptions: {
+        presets: [{plugins: [require('./fixtures/jsx-id-prefix.plugin')]}],
+      },
+    },
   ],
 })
diff --git a/src/index.js b/src/index.js
@@ -52,66 +52,29 @@ function macrosPlugin(babel, {require: _require = require} = {}) {
   return {
     name: 'macros',
     visitor: {
-      ImportDeclaration(path, state) {
-        const isMacros = looksLike(path, {
-          node: {
-            source: {
-              value: v => macrosRegex.test(v),
-            },
-          },
-        })
-        if (!isMacros) {
-          return
-        }
-        const imports = path.node.specifiers.map(s => ({
-          localName: s.local.name,
-          importedName:
-            s.type === 'ImportDefaultSpecifier' ? 'default' : s.imported.name,
-        }))
-        const source = path.node.source.value
-        const result = applyMacros({
-          path,
-          imports,
-          source,
-          state,
-          babel,
-          interopRequire,
-        })
-
-        if (!result || !result.keepImports) {
-          path.remove()
-        }
-      },
-      VariableDeclaration(path, state) {
-        const isMacros = child =>
-          looksLike(child, {
-            node: {
-              init: {
-                callee: {
-                  type: 'Identifier',
-                  name: 'require',
+      Program(progPath, state) {
+        progPath.traverse({
+          ImportDeclaration(path) {
+            const isMacros = looksLike(path, {
+              node: {
+                source: {
+                  value: v => macrosRegex.test(v),
                 },
-                arguments: args =>
-                  args.length === 1 && macrosRegex.test(args[0].value),
               },
-            },
-          })
-
-        path
-          .get('declarations')
-          .filter(isMacros)
-          .forEach(child => {
-            const imports = child.node.id.name
-              ? [{localName: child.node.id.name, importedName: 'default'}]
-              : child.node.id.properties.map(property => ({
-                  localName: property.value.name,
-                  importedName: property.key.name,
-                }))
-
-            const call = child.get('init')
-            const source = call.node.arguments[0].value
+            })
+            if (!isMacros) {
+              return
+            }
+            const imports = path.node.specifiers.map(s => ({
+              localName: s.local.name,
+              importedName:
+                s.type === 'ImportDefaultSpecifier'
+                  ? 'default'
+                  : s.imported.name,
+            }))
+            const source = path.node.source.value
             const result = applyMacros({
-              path: call,
+              path,
               imports,
               source,
               state,
@@ -120,9 +83,52 @@ function macrosPlugin(babel, {require: _require = require} = {}) {
             })
 
             if (!result || !result.keepImports) {
-              child.remove()
+              path.remove()
             }
-          })
+          },
+          VariableDeclaration(path) {
+            const isMacros = child =>
+              looksLike(child, {
+                node: {
+                  init: {
+                    callee: {
+                      type: 'Identifier',
+                      name: 'require',
+                    },
+                    arguments: args =>
+                      args.length === 1 && macrosRegex.test(args[0].value),
+                  },
+                },
+              })
+
+            path
+              .get('declarations')
+              .filter(isMacros)
+              .forEach(child => {
+                const imports = child.node.id.name
+                  ? [{localName: child.node.id.name, importedName: 'default'}]
+                  : child.node.id.properties.map(property => ({
+                      localName: property.value.name,
+                      importedName: property.key.name,
+                    }))
+
+                const call = child.get('init')
+                const source = call.node.arguments[0].value
+                const result = applyMacros({
+                  path: call,
+                  imports,
+                  source,
+                  state,
+                  babel,
+                  interopRequire,
+                })
+
+                if (!result || !result.keepImports) {
+                  child.remove()
+                }
+              })
+          },
+        })
       },
     },
   }
@@ -140,10 +146,8 @@ function applyMacros({path, imports, source, state, babel, interopRequire}) {
     (byName, {importedName, localName}) => {
       const binding = path.scope.getBinding(localName)
 
-      if (binding) {
-        byName[importedName] = binding.referencePaths
-        hasReferences = hasReferences || Boolean(byName[importedName].length)
-      }
+      byName[importedName] = binding.referencePaths
+      hasReferences = hasReferences || Boolean(byName[importedName].length)
 
       return byName
     },