refactor(macro): pass down and manipulate with path instead of node

lingui · Feb 14, 2023 · ec1b9e2 · ec1b9e2
1 parent 77d1c08
commit ec1b9e2
Show file tree

Hide file tree

Showing 4 changed files with 187 additions and 150 deletions.
diff --git a/packages/macro/src/index.ts b/packages/macro/src/index.ts
@@ -52,7 +52,7 @@ const jsxMacroTags = new Set(["Trans", "Plural", "Select", "SelectOrdinal"])
 function macro({ references, state, babel, config }: MacroParams) {
   const opts: LinguiMacroOpts = config as LinguiMacroOpts
 
-  const jsxNodes: NodePath[] = []
+  const jsxNodes = new Set<NodePath>()
   const jsNodes: NodePath[] = []
   let needsI18nImport = false
 
@@ -64,9 +64,13 @@ function macro({ references, state, babel, config }: MacroParams) {
         jsNodes.push(node.parentPath)
       })
     } else if (jsxMacroTags.has(tagName)) {
+      // babel-plugin-macros return JSXIdentifier nodes.
+      // Which is for every JSX element would be presented twice (opening / close)
+      // Here we're taking JSXElement and dedupe it.
+
       nodes.forEach((node) => {
         // identifier.openingElement.jsxElement
-        jsxNodes.push(node.parentPath.parentPath)
+        jsxNodes.add(node.parentPath.parentPath)
       })
     } else {
       throw nodes[0].buildCodeFrameError(`Unknown macro ${tagName}`)
@@ -82,7 +86,9 @@ function macro({ references, state, babel, config }: MacroParams) {
     if (macro.replacePath(path)) needsI18nImport = true
   })
 
-  jsxNodes.filter(isRootPath(jsxNodes)).forEach((path) => {
+  const jsxNodesArray = Array.from(jsxNodes)
+
+  jsxNodesArray.filter(isRootPath(jsxNodesArray)).forEach((path) => {
     if (alreadyVisited(path)) return
     const macro = new MacroJSX(babel, { stripNonEssentialProps })
     macro.replacePath(path)
@@ -92,7 +98,7 @@ function macro({ references, state, babel, config }: MacroParams) {
     addImport(babel, state, i18nImportModule, i18nImportName)
   }
 
-  if (jsxNodes.length) {
+  if (jsxNodes.size) {
     addImport(babel, state, TransImportModule, TransImportName)
   }
 }
@@ -135,6 +141,13 @@ function addImport(
   }
 }
 
+/**
+ * Filtering nested macro calls
+ *
+ * <Macro>
+ *   <Macro /> <-- this would be filtered out
+ * <Macro>
+ */
 function isRootPath(allPath: NodePath[]) {
   return (node: NodePath) =>
     (function traverse(path): boolean {

diff --git a/packages/macro/src/macroJsx.test.ts b/packages/macro/src/macroJsx.test.ts
@@ -1,12 +1,28 @@
-import { parseExpression as _parseExpression } from "@babel/parser"
 import * as types from "@babel/types"
 import MacroJSX, { normalizeWhitespace } from "./macroJsx"
-import { JSXElement } from "@babel/types"
+import { transformSync } from "@babel/core"
+import type { NodePath } from "@babel/traverse"
+import type { JSXElement } from "@babel/types"
 
-const parseExpression = (expression: string) =>
-  _parseExpression(expression, {
-    plugins: ["jsx"],
-  }) as JSXElement
+const parseExpression = (expression: string) => {
+  let path: NodePath<JSXElement>
+
+  transformSync(expression, {
+    filename: "unit-test.js",
+    plugins: [
+      {
+        visitor: {
+          JSXElement: (d) => {
+            path = d
+            d.stop()
+          },
+        },
+      },
+    ],
+  })
+
+  return path
+}
 
 function createMacro() {
   return new MacroJSX({ types }, { stripNonEssentialProps: false })
@@ -302,20 +318,22 @@ describe("jsx macro", () => {
         }),
         format: "plural",
         options: {
-          one: {
-            type: "arg",
-            name: "gender",
-            value: expect.objectContaining({
+          one: [
+            {
+              type: "arg",
               name: "gender",
-              type: "Identifier",
-            }),
-            format: "select",
-            options: {
-              male: "he",
-              female: "she",
-              other: "they",
+              value: expect.objectContaining({
+                name: "gender",
+                type: "Identifier",
+              }),
+              format: "select",
+              options: {
+                male: "he",
+                female: "she",
+                other: "they",
+              },
             },
-          },
+          ],
         },
       })
     })
@@ -332,7 +350,7 @@ describe("jsx macro", () => {
         />`
       )
       const tokens = macro.tokenizeNode(exp)
-      expect(tokens).toMatchObject({
+      expect(tokens[0]).toMatchObject({
         format: "select",
         name: "gender",
         options: {