Repository: lingui/js-lingui
Pull request #1226 (2022-04-05 20:57:03)

fix: JSX attribute parsing issue when using double quotes
- Fixes https://github.com/lingui/js-lingui/issues/1225 

This was the least intrusive way I could come up with by wrapping the relevant `jsxAttribute` string literal value in a `jsxExpressionContainer`. 

This is currently done conditionally in case the message includes a double quote. Note that I believe this could safely be done unconditionally, but it requires updating a lot of tests. I happily do this if we like the suggested solution.

This has been tested out on a fairly big codebase.

vercel[bot]: This pull request is being automatically deployed with Vercel ([learn more](https://vercel.link/github-learn-more)).  
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: [https://vercel.com/lingui-js/js-lingui/4b45GDoiHUkZoH333z3nYLrhfk66](https://vercel.com/lingui-js/js-lingui/4b45GDoiHUkZoH333z3nYLrhfk66)  
✅ Preview: *In Progress*


codecov[bot]: # [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) Report
> Merging [#1226](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) (a0bd4b2) into [main](https://codecov.io/gh/lingui/js-lingui/commit/0e270a394e3317dd243519445feafae8b6e3214c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) (0e270a3) will **increase** coverage by `0.01%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##             main    #1226      +/-   ##
==========================================
+ Coverage   81.66%   81.67%   +0.01%     
==========================================
  Files          56       56              
  Lines        1767     1768       +1     
  Branches      490      491       +1     
==========================================
+ Hits         1443     1444       +1     
  Misses        194      194              
  Partials      130      130              
```


| [Impacted Files](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) | Coverage Δ | |
|---|---|---|
| [packages/macro/src/macroJsx.ts](https://codecov.io/gh/lingui/js-lingui/pull/1226/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui#diff-cGFja2FnZXMvbWFjcm8vc3JjL21hY3JvSnN4LnRz) | `91.24% <100.00%> (+0.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui). Last update [0e270a3...a0bd4b2](https://codecov.io/gh/lingui/js-lingui/pull/1226?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui).

skovhus: @semoal Note that I've been testing this out on a fairly big project and haven't identified any issues.
skovhus: Let me know if I can help fixing the Vercel CI error. 
vercel[bot]: **The latest updates on your projects**. Learn more about [Vercel for Git ↗︎](https://vercel.link/github-learn-more)

| Name | Status | Preview | Updated |
| :--- | :----- | :------ | :------ |
| **js-lingui** | 🔄 Building ([Inspect](https://vercel.com/lingui-js/js-lingui/ASZEpUGwBy7f8dpDMQPhjzmAQEm2)) |  | Apr 20, 2022 at 0:14AM (UTC) |



Patch:
diff --git a/docs/ref/macro.rst b/docs/ref/macro.rst
index 5996cdd0..509e2792 100644
--- a/docs/ref/macro.rst
+++ b/docs/ref/macro.rst
@@ -456,9 +456,9 @@ cardinal plural forms it uses ordinal forms:
 
    import { selectOrdinal } from "@lingui/macro"
    const message = selectOrdinal(count, {
-      one: "1st",
-      two: "2nd",
-      few: "3rd",
+      one: "#st",
+      two: "#nd",
+      few: "#rd",
       other: "#th",
    })
 
@@ -467,7 +467,7 @@ cardinal plural forms it uses ordinal forms:
    import { i18n } from "@lingui/core"
    const message =
    /*i18n*/
-   i18n._('{count, selectOrdinal, one {1st} two {2nd} few {3rd} other {#th}}', {
+   i18n._('{count, selectOrdinal, one {#st} two {#nd} few {#rd} other {#th}}', {
      count
    })
 
@@ -802,9 +802,9 @@ format:
    // count == 4 -> 4th
    <SelectOrdinal
        value={count}
-       one="1st"
-       two="2nd"
-       few="3rd"
+       one="#st"
+       two="#nd"
+       few="#rd"
        other="#th"
    />
 
diff --git a/docs/ref/message-format.rst b/docs/ref/message-format.rst
index 68a0f0d6..b691f24b 100644
--- a/docs/ref/message-format.rst
+++ b/docs/ref/message-format.rst
@@ -47,8 +47,8 @@ Overview
 +------------------+------------------------------------------------------------------+
 | .. icu:: ordinal | .. code-block:: none                                             |
 |                  |                                                                  |
-| Ordinals         |    {count, selectOrdinal, one {1st message}                      |
-|                  |                           two {2nd message}                      |
-|                  |                           few {3rd message}                      |
+| Ordinals         |    {count, selectOrdinal, one {#st message}                      |
+|                  |                           two {#nd message}                      |
+|                  |                           few {#rd message}                      |
 |                  |                           other {#th message}}                   |
 +------------------+------------------------------------------------------------------+
diff --git a/docs/ref/react.rst b/docs/ref/react.rst
index b2adcbb3..01170fe4 100644
--- a/docs/ref/react.rst
+++ b/docs/ref/react.rst
@@ -217,7 +217,7 @@ useLingui
    const CurrentLocale = () => {
       const { i18n } = useLingui()
 
-      return <span>Current locale: {i18n.locale}
+      return <span>Current locale: {i18n.locale}</span>
    }
 
 withI18n
@@ -234,7 +234,7 @@ wrapped component. ``i18n`` object is needed when you have to access the i18n da
    import { withI18n } from "@lingui/react"
 
    const CurrentLocale = withI18n()(({ i18n }) => (
-      <span>Current locale: {i18n.locale}
+      <span>Current locale: {i18n.locale}</span>
    ))
 
 .. _Intl.DateTimeFormat: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/DateTimeFormat
diff --git a/packages/macro/global.d.ts b/packages/macro/global.d.ts
index 41875422..725e097e 100644
--- a/packages/macro/global.d.ts
+++ b/packages/macro/global.d.ts
@@ -119,9 +119,9 @@ declare module "@lingui/macro" {
    * ```
    * import { selectOrdinal } from "@lingui/macro";
    * const message = selectOrdinal(count, {
-   *    one: "1st",
-   *    two: "2nd",
-   *    few: "3rd",
+   *    one: "#st",
+   *    two: "#nd",
+   *    few: "#rd",
    *    other: "#th",
    * });
    * ```
diff --git a/packages/macro/index.d.ts b/packages/macro/index.d.ts
index ec9e66f7..298b979d 100644
--- a/packages/macro/index.d.ts
+++ b/packages/macro/index.d.ts
@@ -111,9 +111,9 @@ export function plural(value: number | string, options: ChoiceOptions): string
  * ```
  * import { selectOrdinal } from "@lingui/macro";
  * const message = selectOrdinal(count, {
- *    one: "1st",
- *    two: "2nd",
- *    few: "3rd",
+ *    one: "#st",
+ *    two: "#nd",
+ *    few: "#rd",
  *    other: "#th",
  * });
  * ```
@@ -172,6 +172,7 @@ export type TransProps = {
   comment?: string
   values?: Record<string, unknown>
   context?: string
+  children?: React.ReactNode
   component?: React.ComponentType<TransRenderProps>
   render?: (props: TransRenderProps) => ReactElement<any, any> | null
 }
diff --git a/packages/macro/src/macroJsx.ts b/packages/macro/src/macroJsx.ts
index f31dd1e5..bddd6547 100644
--- a/packages/macro/src/macroJsx.ts
+++ b/packages/macro/src/macroJsx.ts
@@ -51,6 +51,21 @@ export default class MacroJSX {
     this.elementIndex = makeCounter()
   }
 
+  safeJsxAttribute = (name: string, value: string) => {
+    // Quoted JSX attributes use XML-style escapes instead of JavaScript-style escapes.
+    // This means that <Trans id="Say \"hi\"!" /> is invalid, but <Trans id={"Say \"hi\"!"} /> is valid.
+
+    // We could consider removing this condition and always wrap in a jsxExpressionContainer.
+    const attributeValue = value.includes('"')
+      ? this.types.jsxExpressionContainer(this.types.stringLiteral(value))
+      : this.types.stringLiteral(value)
+
+    return this.types.jsxAttribute(
+      this.types.jsxIdentifier(name),
+      attributeValue
+    )
+  }
+
   replacePath = (path: NodePath) => {
     const tokens = this.tokenizeNode(path.node)
 
@@ -78,21 +93,11 @@ export default class MacroJSX {
 
       if (process.env.NODE_ENV !== "production") {
         if (message) {
-          attributes.push(
-            this.types.jsxAttribute(
-              this.types.jsxIdentifier(MESSAGE),
-              this.types.stringLiteral(message)
-            )
-          )
+          attributes.push(this.safeJsxAttribute(MESSAGE, message))
         }
       }
     } else {
-      attributes.push(
-        this.types.jsxAttribute(
-          this.types.jsxIdentifier(ID),
-          this.types.stringLiteral(message)
-        )
-      )
+      attributes.push(this.safeJsxAttribute(ID, message))
     }
 
     if (process.env.NODE_ENV !== "production") {
diff --git a/packages/react/src/I18nProvider.tsx b/packages/react/src/I18nProvider.tsx
index 2fda3e33..17a42b2c 100644
--- a/packages/react/src/I18nProvider.tsx
+++ b/packages/react/src/I18nProvider.tsx
@@ -12,6 +12,7 @@ export type withI18nProps = {
 
 export type I18nProviderProps = I18nContext & {
   forceRenderOnLocaleChange?: boolean
+  children?: React.ReactNode
 }
 
 const LinguiContext = React.createContext<I18nContext>(null)
diff --git a/packages/react/src/Trans.tsx b/packages/react/src/Trans.tsx
index c58abb18..af379d15 100644
--- a/packages/react/src/Trans.tsx
+++ b/packages/react/src/Trans.tsx
@@ -6,7 +6,7 @@ import { formatElements } from "./format"
 export type TransRenderProps = {
   id?: string
   translation?: React.ReactNode
-  children?: string | any[] | React.ReactNode
+  children?: React.ReactNode
   message?: string | null
 }
 
@@ -17,6 +17,7 @@ export type TransProps = {
   context?: string
   components: { [key: string]: React.ElementType | any }
   formats?: Object
+  children?: React.ReactNode
   component?: React.ComponentType<TransRenderProps>
   render?: (props: TransRenderProps) => React.ReactElement<any, any> | null
 }


Test patch:
diff --git a/packages/cli/src/api/formats/po-gettext.test.ts b/packages/cli/src/api/formats/po-gettext.test.ts
index 44350366..ee6c837a 100644
--- a/packages/cli/src/api/formats/po-gettext.test.ts
+++ b/packages/cli/src/api/formats/po-gettext.test.ts
@@ -343,7 +343,7 @@ msgstr[2] "# dní"
   describe("when using 'selectOrdinal' format", () => {
     const catalog = {
       select_ordinal_message: {
-        message: `{count, selectOrdinal, one {1st} two {2nd} few {3rd} other {#th}}`,
+        message: `{count, selectOrdinal, one {#st} two {#nd} few {#rd} other {#th}}`,
         translation: "",
       },
     }
diff --git a/packages/cli/src/api/pseudoLocalize.test.ts b/packages/cli/src/api/pseudoLocalize.test.ts
index 950a9871..4dd0b187 100644
--- a/packages/cli/src/api/pseudoLocalize.test.ts
+++ b/packages/cli/src/api/pseudoLocalize.test.ts
@@ -71,10 +71,10 @@ describe("PseudoLocalization", () => {
   it("SelectOrdinal", () => {
     expect(
       pseudoLocalize(
-        "{count, selectordinal, offset:1 one {1st} two {2nd} few {3rd} =4 {4th} many {testMany} other {#th}}"
+        "{count, selectordinal, offset:1 one {#st} two {#nd} few {#rd} =4 {4th} many {testMany} other {#th}}"
       )
     ).toEqual(
-      "{count, selectordinal, offset:1 one {1śţ} two {2ńď} few {3ŕď} =4 {4ţĥ} many {ţēśţMàńŷ} other {#ţĥ}}"
+      "{count, selectordinal, offset:1 one {#śţ} two {#ńď} few {#ŕď} =4 {4ţĥ} many {ţēśţMàńŷ} other {#ţĥ}}"
     )
   })
 
diff --git a/packages/core/src/dev/compile.test.ts b/packages/core/src/dev/compile.test.ts
index 0aa4d253..c232ad76 100644
--- a/packages/core/src/dev/compile.test.ts
+++ b/packages/core/src/dev/compile.test.ts
@@ -59,7 +59,7 @@ describe("compile", function () {
 
   it("should compile selectordinal", function () {
     const cache = prepare(
-      "{value, selectordinal, one {1st Book} two {2nd Book}}"
+      "{value, selectordinal, one {#st Book} two {#nd Book}}"
     )
     expect(cache({ value: 1 })).toEqual("1st Book")
     expect(cache({ value: 2 })).toEqual("2nd Book")
diff --git a/packages/macro/test/jsx-trans.ts b/packages/macro/test/jsx-trans.ts
index d5282d18..950f9fc4 100644
--- a/packages/macro/test/jsx-trans.ts
+++ b/packages/macro/test/jsx-trans.ts
@@ -92,6 +92,19 @@ export default [
         }} />;
       `,
   },
+  {
+    name: "Quoted JSX attributes are handled",
+    input: `
+        import { Trans } from '@lingui/macro';
+        <Trans>Speak "friend"!</Trans>;
+        <Trans id="custom-id">Speak "friend"!</Trans>;
+      `,
+    expected: `
+        import { Trans } from "@lingui/react";
+        <Trans id={'Speak "friend"!'} />;
+        <Trans id="custom-id" message={'Speak "friend"!'} />;
+      `,
+  },
   {
     name: "Template literals as children",
     input: `
diff --git a/packages/macro/test/types.ts b/packages/macro/test/types.ts
index 108de60e..c5957f0c 100644
--- a/packages/macro/test/types.ts
+++ b/packages/macro/test/types.ts
@@ -44,8 +44,8 @@ select(value, {
 
 /** Select Ordinal test */
 selectOrdinal(value, {
-  one: "1st",
-  two: "2nd",
-  few: "3rd",
+  one: "#st",
+  two: "#nd",
+  few: "#rd",
   other: "#th",
 })