Repository: lingui/js-lingui
Pull request #1141 (2021-09-28 20:36:35)

fix(types): correct types for  macro with custom i18n instance
This PR fixes an issue introduced in #1139, that would cause a type error to be raised, even if the usage was correct.

I'm not sure how I didn't notice this before, but I guess it's because either type checking was disabled while I tested it, or because the files were JS only.

The issue is that for TypeScript, doing ```t(x)`template ${string}`;```, is seen as two individual calls (`result = t(x)` and ```result`template ${string}`;```), so it also needs to be typed like that, even though the macro itself will never let it be executed as such.

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/E11UqqWV89JAyvEBmeXv6Zuqdzj1](https://vercel.com/lingui-js/js-lingui/E11UqqWV89JAyvEBmeXv6Zuqdzj1)  
✅ Preview: *In Progress*


jeroenvisser101: I'm not sure if this is something we want to have, but we could add https://github.com/Microsoft/dtslint and add assertions for correct/wrong usage of the macro, to avoid this in the future, but I'm not sure if it's useful, given that the only hand-written type definitions are in the macro anyway.
semoal: Yep, just noticed the same.

Created a playground on TypeScript website for checking this, I'm going to create a test-types.ts where I'll add some tests to macros types and some conflictive typings.

There's one case where it fails: `t(i18n)(messageDescriptor)`, basically this use case is when using `defineMessage` and the custom instance of `i18n` (it should work I guess)

[Playground](https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBDAnmApnA3nAsigzrgQwHMUARPAYymDBmgBo4BJARgA4A7OAXzgDMoEEHABEAAQA2wDkQCuwAPQVoKEQCg1CgFRa1cLXAAqUAh1wSCMPHAJwQeQiTgATStVrQ9u-XDEoAHgTgEihecAAGkWGgkLAYCDz8gsLiUjLyCiAEVBAiANxhymbw9vjEaAC8CAAU6GFwcMDOAFyiILhEAHQAFigSErn09XDKIPYcMK0iAOJQKCgw0kS4NvAwvXDdQihg5SJDPg2ljiit4QASfQNwACToHEEo3OEHBtwAlAU+keFeYX6BYKhb5RHwxaDwTDwXgCISiSRLDJZHL5QoQYp2BzlOBVGC1YZNKbtLpgCSyEwSfbDY7lVqk8kECTVABujNkKEYmHRp1EnT5+zgEHWKCgUz5nREPHerylXwMPz+PjEuxMwhpJHIuCoNEW0CMG1cWvcdCgCAgCBMZgsVi8CjUrgoFjm-FkHAouq4ePVZDcOuA0FaODKGt9Hig71auBg1Bkmh0YWMpnMlmstiswRTcCjMaIcFkuCWCA2RAGACNGcx2FxpFHTBRgd4DACgqSGxFQQZwXEoYlYSkEelFMjBKifEUo5jg5UEBcrua7g97M85e3fvo7Q6nWg+K73f7PdU9HApFYKbhWoYUBmrABlaNLXAAQSgJkQBzg4tJ2RQWwkrig542BwiAANoALpqBGWb3rG2iNkYlrJlYKxpleX5WNBOZ5gWMg2HARDAMyKBcKwnCNMUdYNv8AQtiEYQKmC4AQvE0JJHCqSIkO2QjiuXaQpWZEwsk8JpHIijKHMo4GOO8DAFWOJwBwKAAO4CRw+KHMeEAUIyPIiBwlIykcWIkIBmAiJc-TmugAAM3AiFM5yMtctn2TwMofCuMmTicCl4nJnDvLOVm3PcjzLvRHYbigjoEM6O5uh6NQBRwrSkRwUHVCeIqMoBl7Xigd45k+L4EG+H58l+9a-v+gGmKBYHvDiAB8mFLBoajeSlaXyVU6C8AQKzpRo3nIZM2AmT6Ro6nqfVHoSohjVSxlTlMS1DNwnXohOrJktOIhLRocEzuEdjceaY2FlodowOEl5Rr8t33fAdy7eyzxqHiY3vJ91QpUFY2PX9VYA3gL3oG9TxA-91TfRoQA)
codecov[bot]: # [Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1141?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) Report
> Merging [#1141](https://codecov.io/gh/lingui/js-lingui/pull/1141?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) (c222772) into [main](https://codecov.io/gh/lingui/js-lingui/commit/89211567632733cf9955cafc9c92bd87c6154852?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui) (8921156) will **not change** coverage.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/lingui/js-lingui/pull/1141/graphs/tree.svg?width=650&height=150&src=pr&token=CuJ1VD5BY5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui)](https://codecov.io/gh/lingui/js-lingui/pull/1141?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui)

```diff
@@           Coverage Diff           @@
##             main    #1141   +/-   ##
=======================================
  Coverage   82.28%   82.28%           
=======================================
  Files          56       56           
  Lines        1699     1699           
  Branches      466      466           
=======================================
  Hits         1398     1398           
  Misses        176      176           
  Partials      125      125           
```



------

[Continue to review full report at Codecov](https://codecov.io/gh/lingui/js-lingui/pull/1141?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/1141?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=lingui). Last update [47aa68a...c222772](https://codecov.io/gh/lingui/js-lingui/pull/1141?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).

semoal: > Yep, just noticed the same.
> 
> Created a playground on TypeScript website for checking this, I'm going to create a test-types.ts where I'll add some tests to macros types and some conflictive typings.
> 
> There's one case where it fails: `t(i18n)(messageDescriptor)`, basically this use case is when using `defineMessage` and the custom instance of `i18n` (it should work I guess)
> 
> [Playground](https://www.typescriptlang.org/play?#code/JYWwDg9gTgLgBDAnmApnA3nAsigzrgQwHMUARPAYymDBmgBo4BJARgA4A7OAXzgDMoEEHABEAAQA2wDkQCuwAPQVoKEQCg1CgFRa1cLXAAqUAh1wSCMPHAJwQeQiTgATStVrQ9u-XDEoAHgTgEihecAAGkWGgkLAYCDz8gsLiUjLyCiAEVBAiANxhymbw9vjEaAC8CAAU6GFwcMDOAFyiILhEAHQAFigSErn09XDKIPYcMK0iAOJQKCgw0kS4NvAwvXDdQihg5SJDPg2ljiit4QASfQNwACToHEEo3OEHBtwAlAU+keFeYX6BYKhb5RHwxaDwTDwXgCISiSRLDJZHL5QoQYp2BzlOBVGC1YZNKbtLpgCSyEwSfbDY7lVqk8kECTVABujNkKEYmHRp1EnT5+zgEHWKCgUz5nREPHerylXwMPz+PjEuxMwhpJHIuCoNEW0CMG1cWvcdCgCAgCBMZgsVi8CjUrgoFjm-FkHAouq4ePVZDcOuA0FaODKGt9Hig71auBg1Bkmh0YWMpnMlmstiswRTcCjMaIcFkuCWCA2RAGACNGcx2FxpFHTBRgd4DACgqSGxFQQZwXEoYlYSkEelFMjBKifEUo5jg5UEBcrua7g97M85e3fvo7Q6nWg+K73f7PdU9HApFYKbhWoYUBmrABlaNLXAAQSgJkQBzg4tJ2RQWwkrig542BwiAANoALpqBGWb3rG2iNkYlrJlYKxpleX5WNBOZ5gWMg2HARDAMyKBcKwnCNMUdYNv8AQtiEYQKmC4AQvE0JJHCqSIkO2QjiuXaQpWZEwsk8JpHIijKHMo4GOO8DAFWOJwBwKAAO4CRw+KHMeEAUIyPIiBwlIykcWIkIBmAiJc-TmugAAM3AiFM5yMtctn2TwMofCuMmTicCl4nJnDvLOVm3PcjzLvRHYbigjoEM6O5uh6NQBRwrSkRwUHVCeIqMoBl7Xigd45k+L4EG+H58l+9a-v+gGmKBYHvDiAB8mFLBoajeSlaXyVU6C8AQKzpRo3nIZM2AmT6Ro6nqfVHoSohjVSxlTlMS1DNwnXohOrJktOIhLRocEzuEdjceaY2FlodowOEl5Rr8t33fAdy7eyzxqHiY3vJ91QpUFY2PX9VYA3gL3oG9TxA-91TfRoQA)

Ah, never mind about the `messageDescriptor` issue, I think we can fix it like this:

```ts
declare function t(i18n: I18n): (messageDescriptior: MessageDescriptor | TemplateStringsArray, ...placeholders: unknown[]) => string
```

Take a look to the playground and tell me wdyt
jeroenvisser101: Ah nice catch, although I think the best way to do that is with overloads (so declaring the function multiple times. This disallows or errors when additional parameters are passed (placeholders, in this case, which doesn't make sense there).

Do you want to also change any -> unknown? I don't think it makes any difference here, as those types are only for the implementation, not for consuming the passed arguments in Lingui itself.
jeroenvisser101: Oh btw, I'm not sure if `t(i18n)(messageDescriptor)` works, I didn't implement it as such, so if it works, it's by accident
semoal: > Oh btw, I'm not sure if `t(i18n)(messageDescriptor)` works, I didn't implement it as such, so if it works, it's by accident

Not sure too, I'll check it tomorrow in detail and we adjust the typing accordingly, there's no hurry at all.
jeroenvisser101: > Not sure too, I'll check it tomorrow in detail and we adjust the typing accordingly, there's no hurry at all.

I think I have a fix for both the types, and adding support for `t(i18n)(messageDescriptor)` (and a small typo fix in the argument name in the types), no rush though :)
jeroenvisser101: Better now, [example playground here](https://www.typescriptlang.org/play?ssl=73&ssc=4&pln=70&pc=6#code/JYWwDg9gTgLgBDAnmApnA3nAsigzrgQwHMUARPAYymDBmgBo4BJARgA4A7OAXzgDMoEEHABEAAQA2wDkQCuwAPQVoKEQCgNcOAoBUOtVrg64AFSgEOuCQRh44BOCDyEScACaVqtaAa37DxmIoAB4E4BIovkZwAAZxUcagkLAYCDz8gsLiUjLyCiAEVBAiANwJcMqW8E74xGgAvAgAFOjlWsBuAFyiILhEAHQAFigSEsX0bRVCThww3SIA4lAoKDDSRLj28DDDcINCKGB1IhMBhjUuKN0xABIjY3AAJOgcYSjcMad+PACUZWdxGIJcpBULhSIA+JnJLQeCYeC8ARCUSSdZ5ApFUrlSq4arOOpwRowFqTDrzXoDMASWTmCQnSYXOrdKk0ggSJoANzZshQjEwEA4V1E-RFJzgEB2KCg8xF-REvy+0W4f3KgOBZzER3Mwg8uCoNDoUFMu0Zrl1+u8RroCHMlmstgSCl8Hgo1mW-FkHAoawFzXNXkN3RwtRI5D1AegP26uOoMk02j05TMFisNjsDls4TTcBj6zgslweclcCIYwARmzmOwuNJcRYKBC-CCQmEqY3omroeBYakERlkdk0YoMYIsWccXiQw0ELd7hAni83h9-t9O34nVoXW60HxPd7gL7iVE4FJbLTcN0TCgs7YAMowWMbACCUHMiEVcFlVMKKH2Eg8UAXvYHCIAA2gAur4UY5g+6zxro-jfMmdpppsGbXt+tgwY+4pGqaaD+ga0D5oWMj2CWwAcigXCsJwcC1jA9btoh0Sgq2ESqlC3wwik8LpEiWSorkw6FKOK7RDxcJVnRiKZCiORyIoyjLGO3wTvR1aEnAgoAO7SRwJJnFoYwUGyQoiBwdKKsYWj4UBmAiHcozzugAAM3AiPMNxsg8bkeTw1m-OJxjqfhWnEsA1Y-LOzkLq8TjLpxQIai24JJeUkm9vxcmDsJ+SicUwX0d2vH6dlA5CYpSgqKp0TqZFdGNLp+mGd8xkQKZETzJZ9JGY4+IkPZohOb57mecNPkuWNAXlMqRWhQN04RVFLT9VO1wjfOzzxe8MRBelAQbu4KCugQ7q7l6PpcEehgaZw3S0RwUHdK0t1NKeUpskBV43ig96PrgL5vowX7WA2f4AUBFhgeB0G5nGb2EZaQaLWGFqGnDsEIzwGgaPV1YPZpjToLwBCbI9eMCriCB4HM2Co54RFGsTvhkqIti4r1a2XPMHMwPS3BqOpXLUtOIh8+oagITOe0jvOfN5joTowDEV64kCKtq-AzwizyHxqMSfM-AbTQNRw0V8xrptRTEfMLrru0m2bPxNEbGhAA)

Patch:
diff --git a/docs/ref/macro.rst b/docs/ref/macro.rst
index 0b0b8d39..5996cdd0 100644
--- a/docs/ref/macro.rst
+++ b/docs/ref/macro.rst
@@ -146,6 +146,13 @@ Examples of JS macros
 +-------------------------------------------------------------+--------------------------------------------------------------------+
 | .. code-block:: js                                          | .. code-block:: js                                                 |
 |                                                             |                                                                    |
+|    t(customI18n)({                                          |    customI18n._(/*i18n*/{                                          |
+|       id: "msg.refresh",                                    |      id: "msg.refresh",                                            |
+|       message: "Refresh inbox"                              |      message: "Refresh inbox"                                      |
+|    })                                                       |    })                                                              |
++-------------------------------------------------------------+--------------------------------------------------------------------+
+| .. code-block:: js                                          | .. code-block:: js                                                 |
+|                                                             |                                                                    |
 |    defineMessage({                                          |    /*i18n*/{                                                       |
 |       id: "msg.refresh",                                    |      id: "msg.refresh",                                            |
 |       message: "Refresh inbox"                              |      message: "Refresh inbox"                                      |
diff --git a/packages/macro/global.d.ts b/packages/macro/global.d.ts
index 25a92329..01aada34 100644
--- a/packages/macro/global.d.ts
+++ b/packages/macro/global.d.ts
@@ -28,9 +28,9 @@ declare module "@lingui/macro" {
    * });
    * ```
    *
-   * @param messageDescriptior The descriptor to translate
+   * @param descriptor The message descriptor to translate
    */
-  export function t(messageDescriptior: MessageDescriptor): string
+  export function t(descriptor: MessageDescriptor): string
 
   /**
    * Translates a template string using the global I18n instance
@@ -47,7 +47,7 @@ declare module "@lingui/macro" {
   ): string
 
   /**
-   * Translates a template string using a given I18n instance
+   * Translates a template string or message descriptor using a given I18n instance
    *
    * @example
    * ```
@@ -59,12 +59,24 @@ declare module "@lingui/macro" {
    * });
    * const message = t(i18n)`Hello ${name}`;
    * ```
+   *
+   * @example
+   * ```
+   * import { t } from "@lingui/macro";
+   * import { I18n } from "@lingui/core";
+   * const i18n = new I18n({
+   *   locale: "nl",
+   *   messages: { "Hello {0}": "Hallo {0}" },
+   * });
+   * const message = t(i18n)({ message: `Hello ${name}` });
+   * ```
    */
   export function t(
-    i18n: I18n,
-    literals: TemplateStringsArray,
-    ...placeholders: any[]
-  ): string
+    i18n: I18n
+  ): {
+    (literals: TemplateStringsArray, ...placeholders: any[]): string
+    (descriptor: MessageDescriptor): string
+  }
 
   export type UnderscoreDigit<T = string> = { [digit: string]: T }
   export type ChoiceOptions<T = string> = {
diff --git a/packages/macro/index.d.ts b/packages/macro/index.d.ts
index 0a0ffe3e..c5e1fde1 100644
--- a/packages/macro/index.d.ts
+++ b/packages/macro/index.d.ts
@@ -34,9 +34,9 @@ export type ChoiceOptions<T = string> = {
  * });
  * ```
  *
- * @param messageDescriptior The descriptor to translate
+ * @param descriptor The message descriptor to translate
  */
-export function t(messageDescriptior: MessageDescriptor): string
+export function t(descriptor: MessageDescriptor): string
 
 /**
  * Translates a template string using the global I18n instance
@@ -53,7 +53,7 @@ export function t(
 ): string
 
 /**
- * Translates a template string using a given I18n instance
+ * Translates a template string or message descriptor using a given I18n instance
  *
  * @example
  * ```
@@ -65,12 +65,24 @@ export function t(
  * });
  * const message = t(i18n)`Hello ${name}`;
  * ```
+ *
+ * @example
+ * ```
+ * import { t } from "@lingui/macro";
+ * import { I18n } from "@lingui/core";
+ * const i18n = new I18n({
+ *   locale: "nl",
+ *   messages: { "Hello {0}": "Hallo {0}" },
+ * });
+ * const message = t(i18n)({ message: `Hello ${name}` });
+ * ```
  */
 export function t(
-  i18n: I18n,
-  literals: TemplateStringsArray,
-  ...placeholders: any[]
-): string
+  i18n: I18n
+): {
+  (literals: TemplateStringsArray, ...placeholders: any[]): string
+  (descriptor: MessageDescriptor): string
+}
 
 /**
  * Pluralize a message
diff --git a/packages/macro/src/macroJs.ts b/packages/macro/src/macroJs.ts
index d840f4e4..189bdbd0 100644
--- a/packages/macro/src/macroJs.ts
+++ b/packages/macro/src/macroJs.ts
@@ -128,6 +128,18 @@ export default class MacroJs {
       return false
     }
 
+    // t(i18nInstance)(messageDescriptor) -> i18nInstance._(messageDescriptor)
+    if (
+      this.types.isCallExpression(path.node) &&
+      this.types.isCallExpression(path.parentPath.node) &&
+      this.types.isIdentifier(path.node.arguments[0]) &&
+      this.isIdentifier(path.node.callee, "t")
+    ) {
+      const i18nInstance = path.node.arguments[0]
+      this.replaceTAsFunction(path.parentPath, i18nInstance)
+      return false
+    }
+
     if (
       this.types.isCallExpression(path.node) &&
       this.isIdentifier(path.node.callee, "t")
@@ -185,11 +197,11 @@ export default class MacroJs {
    * macro `t` is called with MessageDescriptor, after that
    * we create a new node to append it to i18n._
    */
-  replaceTAsFunction = (path) => {
+  replaceTAsFunction = (path, linguiInstance?: babelTypes.Identifier) => {
     const descriptor = this.processDescriptor(path.node.arguments[0])
     const newNode = this.types.callExpression(
       this.types.memberExpression(
-        this.types.identifier(this.i18nImportName),
+        linguiInstance ?? this.types.identifier(this.i18nImportName),
         this.types.identifier("_")
       ),
       [descriptor]


Test patch:
diff --git a/packages/macro/test/js-t.ts b/packages/macro/test/js-t.ts
index 6da76684..60884191 100644
--- a/packages/macro/test/js-t.ts
+++ b/packages/macro/test/js-t.ts
@@ -139,6 +139,25 @@ export default [
         });
       `,
   },
+  {
+    name:
+      "Support template strings in t macro message, with custom i18n instance",
+    input: `
+        import { t } from '@lingui/macro'
+        import { i18n } from './lingui'
+        const msg = t(i18n)({ message: \`Hello \${name}\` })
+      `,
+    expected: `import { i18n } from "./lingui";
+    const msg =
+      i18n._(/*i18n*/
+        {
+          id: "Hello {name}",
+          values: {
+            name: name,
+          },
+        });
+      `,
+  },
   {
     name: "Support id and comment in t macro as callExpression",
     input: `