Repository: lingui/js-lingui (ts)

Issue:
Macro `Plural` are not extracted correctly with `component` prop
**Describe the bug**
If marco `<Plural>` is passed with `component` prop, this prop will also be extracted to message catalog.

**To Reproduce**

```tsx
import { Plural } from "@lingui/macro"

export default function App() {
  const count = 1
  const Div = ({ children }) => <div>{children}</div>

  return (
    <Plural
      id="message"
      value={count}
      _0="No messages"
      one="# message"
      other="# messages"
      component={Div}
    />
  )
}
```

This code will extracts incorrect message like follows:

```json
{
  "message": {
    "translation": "{count, plural, =0 {No messages} one {# message} other {# messages} component {{Div}}}",
    "message": "{count, plural, =0 {No messages} one {# message} other {# messages} component {{Div}}}",
    "extractedComments": []
  }
}
```

Please notice that ``component {{Div}}`` is appended to `message.translation` and `message.message`.

**Expected behavior**
The `component` prop should be ignored.

**Additional context**
Add any other context about the problem here.

- jsLingui version `3.4.0`
- Babel version `@babel/core@7.12.10`



Pull request:
fix: <Select /> macro not extracted properly
#919 Will close this issue

Also will close #959

Patch:
diff --git a/packages/macro/src/macroJsx.ts b/packages/macro/src/macroJsx.ts
index 3a0d8603..c6646601 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)),
     }
   }
 
@@ -227,10 +228,10 @@ export default class MacroJSX {
         }
       } else if (this.types.isTemplateLiteral(exp)) {
         const tokenize = R.pipe(
-          // Don't output tokens without text.
+          // Don"t output tokens without text.
           R.evolve({
             quasis: R.map((text: babelTypes.TemplateElement) => {
-              // Don't output tokens without text.
+              // Don"t output tokens without text.
               const value = text.value.raw
               if (value === "") return null
 
@@ -268,7 +269,15 @@ 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,
+      // we remove <Trans /> react props that are not useful for translation
+      "render",
+      "component",
+      "components"
+    ], true))
 
     const token = {
       type: "arg",
@@ -294,14 +303,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
+        }
       }
     }
 
@@ -348,7 +361,7 @@ export default class MacroJSX {
 
   /**
    * We clean '//\` ' to just '`'
-   */
+   * */
   clearBackslashes(value: string) {
     return value.replace(removeExtraScapedLiterals, "`")
   }


Test patch:
diff --git a/packages/cli/src/api/pseudoLocalize.test.ts b/packages/cli/src/api/pseudoLocalize.test.ts
index c17382ec..2201d1b0 100644
--- a/packages/cli/src/api/pseudoLocalize.test.ts
+++ b/packages/cli/src/api/pseudoLocalize.test.ts
@@ -43,7 +43,7 @@ describe("PseudoLocalization", () => {
           "{count, plural, offset:1 zero {There are no messages} other {There are # messages in your inbox}}"
         )
       ).toEqual(
-        "{count, plural, offset:1 zero {Ţĥēŕē'ŕē ńō mēśśàĝēś} other {Ţĥēŕē'ŕē # mēśśàĝēś ĩń ŷōũŕ ĩńƀōx}}"
+        "{count, plural, offset:1 zero {Ţĥēŕē àŕē ńō mēśśàĝēś} other {Ţĥēŕē àŕē # mēśśàĝēś ĩń ŷōũŕ ĩńƀōx}}"
       )
     })
 
@@ -53,7 +53,7 @@ describe("PseudoLocalization", () => {
           "{count, plural, zero {There's # <span>message</span>} other {There are # messages}"
         )
       ).toEqual(
-        "{count, plural, zero {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē'ŕē # mēśśàĝēś}"
+        "{count, plural, zero {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}"
       )
     })
 
@@ -63,7 +63,7 @@ describe("PseudoLocalization", () => {
           "{count, plural, =0 {There's # <span>message</span>} other {There are # messages}"
         )
       ).toEqual(
-        "{count, plural, =0 {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē'ŕē # mēśśàĝēś}"
+        "{count, plural, =0 {Ţĥēŕē'ś # <span>mēśśàĝē</span>} other {Ţĥēŕē àŕē # mēśśàĝēś}"
       )
     })
   })
diff --git a/packages/macro/src/macroJs.test.ts b/packages/macro/src/macroJs.test.ts
index ebecce94..00957532 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 c1bd60b6..47abf011 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",
+        })
+      })
+    })
   })
 })