From c3b2783b49cf236e8752c88fdb98fea4489b555c Mon Sep 17 00:00:00 2001
From: Sergio Moreno <sergio.moreno@flowable.com>
Date: Tue, 26 Jan 2021 12:27:04 +0100
Subject: [PATCH] fix: <Select /> choices not extracted

---
 packages/macro/src/macroJs.test.ts  | 62 ++++++++++++++++++++++-----
 packages/macro/src/macroJsx.test.ts | 65 +++++++++++++++++++++++------
 packages/macro/src/macroJsx.ts      | 34 +++++++++------
 3 files changed, 126 insertions(+), 35 deletions(-)

diff --git a/packages/macro/src/macroJs.test.ts b/packages/macro/src/macroJs.test.ts
index ebecce94b..009575324 100644
--- a/packages/macro/src/macroJs.test.ts
+++ b/packages/macro/src/macroJs.test.ts
@@ -6,9 +6,9 @@ function createMacro() {
   return new MacroJs({ types }, { i18nImportName: "i18n" })
 }
 
-describe("js macro", function () {
-  describe("tokenizeTemplateLiteral", function () {
-    it("simple message without arguments", function () {
+describe("js macro", () => {
+  describe("tokenizeTemplateLiteral", () => {
+    it("simple message without arguments", () => {
       const macro = createMacro()
       const exp = parseExpression("t`Message`")
       const tokens = macro.tokenizeTemplateLiteral(exp)
@@ -20,7 +20,7 @@ describe("js macro", function () {
       ])
     })
 
-    it("message with named argument", function () {
+    it("message with named argument", () => {
       const macro = createMacro()
       const exp = parseExpression("t`Message ${name}`")
       const tokens = macro.tokenizeTemplateLiteral(exp)
@@ -40,7 +40,7 @@ describe("js macro", function () {
       ])
     })
 
-    it("message with positional argument", function () {
+    it("message with positional argument", () => {
       const macro = createMacro()
       const exp = parseExpression("t`Message ${obj.name}`")
       const tokens = macro.tokenizeTemplateLiteral(exp)
@@ -59,7 +59,7 @@ describe("js macro", function () {
       ])
     })
 
-    it("message with plural", function () {
+    it("message with plural", () => {
       const macro = createMacro()
       const exp = parseExpression("t`Message ${plural(count, {})}`")
       const tokens = macro.tokenizeTemplateLiteral(exp)
@@ -81,8 +81,8 @@ describe("js macro", function () {
     })
   })
 
-  describe("tokenizeChoiceMethod", function () {
-    it("plural", function () {
+  describe("tokenizeChoiceMethod", () => {
+    it("plural", () => {
       const macro = createMacro()
       const exp = parseExpression(
         "plural(count, { one: '# book', other: '# books'})"
@@ -103,7 +103,7 @@ describe("js macro", function () {
       })
     })
 
-    it("plural with offset", function () {
+    it("plural with offset", () => {
       const macro = createMacro()
       const exp = parseExpression(
         `plural(count, {
@@ -131,7 +131,7 @@ describe("js macro", function () {
       })
     })
 
-    it("plural with template literal", function () {
+    it("plural with template literal", () => {
       const macro = createMacro()
       const exp = parseExpression(
         "plural(count, { one: `# glass of ${drink}`, other: `# glasses of ${drink}`})"
@@ -178,7 +178,7 @@ describe("js macro", function () {
       })
     })
 
-    it("plural with select", function () {
+    it("plural with select", () => {
       const macro = createMacro()
       const exp = parseExpression(
         `plural(count, {
@@ -218,5 +218,45 @@ describe("js macro", function () {
         },
       })
     })
+
+    it("select", () => {
+      const macro = createMacro()
+      const exp = parseExpression(
+        `select(gender, {
+          male: "he",
+          female: "she",
+          other: "they"
+        })`
+      )
+      const tokens = macro.tokenizeChoiceComponent(exp)
+      expect(tokens).toEqual({
+        format: "select",
+        name: "gender",
+        options: expect.objectContaining({
+          female: "she",
+          male: "he",
+          offset: undefined,
+          other: "they"
+        }),
+        type: "arg",
+        value: {
+          end: 13,
+          loc: {
+            end: expect.objectContaining({
+              column: 13,
+              line: 1,
+            }),
+            identifierName: "gender",
+            start: expect.objectContaining({
+              column: 7,
+              line: 1,
+            }),
+          },
+          name: "gender",
+          start: 7,
+          type: "Identifier",
+        }
+      })
+    })
   })
 })
diff --git a/packages/macro/src/macroJsx.test.ts b/packages/macro/src/macroJsx.test.ts
index c1bd60b67..47abf0111 100644
--- a/packages/macro/src/macroJsx.test.ts
+++ b/packages/macro/src/macroJsx.test.ts
@@ -11,9 +11,9 @@ function createMacro() {
   return new MacroJSX({ types })
 }
 
-describe("jsx macro", function () {
-  describe("tokenizeTrans", function () {
-    it("simple message without arguments", function () {
+describe("jsx macro", () => {
+  describe("tokenizeTrans", () => {
+    it("simple message without arguments", () => {
       const macro = createMacro()
       const exp = parseExpression("<Trans>Message</Trans>")
       const tokens = macro.tokenizeTrans(exp)
@@ -25,7 +25,7 @@ describe("jsx macro", function () {
       ])
     })
 
-    it("message with named argument", function () {
+    it("message with named argument", () => {
       const macro = createMacro()
       const exp = parseExpression("<Trans>Message {name}</Trans>")
       const tokens = macro.tokenizeTrans(exp)
@@ -45,7 +45,7 @@ describe("jsx macro", function () {
       ])
     })
 
-    it("message with positional argument", function () {
+    it("message with positional argument", () => {
       const macro = createMacro()
       const exp = parseExpression("<Trans>Message {obj.name}</Trans>")
       const tokens = macro.tokenizeTrans(exp)
@@ -64,7 +64,7 @@ describe("jsx macro", function () {
       ])
     })
 
-    it("message with plural", function () {
+    it("message with plural", () => {
       const macro = createMacro()
       const exp = parseExpression(
         "<Trans>Message <Plural value={count} /></Trans>"
@@ -88,8 +88,8 @@ describe("jsx macro", function () {
     })
   })
 
-  describe("tokenizeChoiceComponent", function () {
-    it("plural", function () {
+  describe("tokenizeChoiceComponent", () => {
+    it("plural", () => {
       const macro = createMacro()
       const exp = parseExpression(
         "<Plural value={count} one='# book' other='# books' />"
@@ -110,7 +110,7 @@ describe("jsx macro", function () {
       })
     })
 
-    it("plural with offset", function () {
+    it("plural with offset", () => {
       const macro = createMacro()
       const exp = parseExpression(
         `<Plural
@@ -139,7 +139,7 @@ describe("jsx macro", function () {
       })
     })
 
-    it("plural with template literal", function () {
+    it("plural with template literal", () => {
       const macro = createMacro()
       const exp = parseExpression(
         "<Plural value={count} one={`# glass of ${drink}`} other={`# glasses of ${drink}`} />"
@@ -186,11 +186,11 @@ describe("jsx macro", function () {
       })
     })
 
-    it("plural with select", function () {
+    it("plural with select", () => {
       const macro = createMacro()
       const exp = parseExpression(
         `<Plural
-          value={count} 
+          value={count}
           one={
             <Select
               value={gender}
@@ -228,5 +228,46 @@ describe("jsx macro", function () {
         },
       })
     })
+
+    it("Select", () => {
+      const macro = createMacro()
+      const exp = parseExpression(
+        `<Select
+          value={gender}
+          male="he"
+          one="heone"
+          female="she"
+          other="they"
+        />`
+      )
+      const tokens = macro.tokenizeNode(exp)
+      expect(tokens).toEqual({
+        format: "select",
+        name: "gender",
+        options: expect.objectContaining({
+          female: "she",
+          male: "he",
+          offset: undefined,
+          other: "they"
+        }),
+        type: "arg",
+        value: expect.objectContaining({
+          "end": 31,
+          "loc":  {
+            "end": {
+              "column": 23,
+              "line": 2,
+            },
+            "identifierName": "gender",
+            "start":  {
+              "column": 17,
+              "line": 2,
+            },
+          },
+          name: "gender",
+          type: "Identifier",
+        })
+      })
+    })
   })
 })
diff --git a/packages/macro/src/macroJsx.ts b/packages/macro/src/macroJsx.ts
index 3a0d86039..bf32ee6f0 100644
--- a/packages/macro/src/macroJsx.ts
+++ b/packages/macro/src/macroJsx.ts
@@ -157,16 +157,17 @@ export default class MacroJSX {
     path.replaceWith(newNode)
   }
 
+  attrName = (names, exclude = false) => {
+    const namesRe = new RegExp("^(" + names.join("|") + ")$")
+    return (attr) =>
+      exclude ? !namesRe.test(attr.name.name) : namesRe.test(attr.name.name)
+  }
+
   stripMacroAttributes = (node) => {
     const { attributes } = node.openingElement
-    const attrName = (names, exclude = false) => {
-      const namesRe = new RegExp("^(" + names.join("|") + ")$")
-      return (attr) =>
-        exclude ? !namesRe.test(attr.name.name) : namesRe.test(attr.name.name)
-    }
-    const id = attributes.filter(attrName([ID]))[0]
-    const message = attributes.filter(attrName([MESSAGE]))[0]
-    const comment = attributes.filter(attrName([COMMENT]))[0]
+    const id = attributes.filter(this.attrName([ID]))[0]
+    const message = attributes.filter(this.attrName([MESSAGE]))[0]
+    const comment = attributes.filter(this.attrName([COMMENT]))[0]
 
     let reserved = [ID, MESSAGE, COMMENT]
     if (this.isI18nComponent(node)) {
@@ -191,7 +192,7 @@ export default class MacroJSX {
       id: maybeNodeValue(id),
       message: maybeNodeValue(message),
       comment: maybeNodeValue(comment),
-      attributes: attributes.filter(attrName(reserved, true)),
+      attributes: attributes.filter(this.attrName(reserved, true)),
     }
   }
 
@@ -268,7 +269,12 @@ export default class MacroJSX {
   tokenizeChoiceComponent = (node) => {
     const element = node.openingElement
     const format = element.name.name.toLowerCase()
-    const props = element.attributes
+    const props = element.attributes.filter(this.attrName([
+      ID,
+      COMMENT,
+      MESSAGE,
+      'render',
+    ], true))
 
     const token = {
       type: "arg",
@@ -294,14 +300,18 @@ export default class MacroJSX {
         token.options.offset = this.types.isStringLiteral(attr.value)
           ? attr.value.value
           : attr.value.expression.value
-      } else if (pluralRuleRe.test(name)) {
+      } else {
         let value
         if (this.types.isStringLiteral(attr.value)) {
           value = attr.value.extra.raw.replace(/(["'])(.*)\1/, "$2")
         } else {
           value = this.tokenizeChildren(attr.value)
         }
-        token.options[jsx2icuExactChoice(name)] = value
+        if (pluralRuleRe.test(name)) {
+          token.options[jsx2icuExactChoice(name)] = value
+        } else {
+          token.options[name] = value
+        }
       }
     }