fix(types): correct types for macro with custom i18n instance

[jeroenvisser101]

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]

This pull request is being automatically deployed with Vercel (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/9UdqQB48LVcvLhMmxiAfUUzPTdZ6
✅ Preview: https://js-lingui-git-fork-jeroenvisser101-jv-fix-type-f644b0-lingui-js.vercel.app

[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

[codecov]

Codecov Report

Merging #1141 (c222772) into main (8921156) will not change coverage.
The diff coverage is n/a.

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 47aa68a...c222772. Read the comment docs.

[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

Ah, never mind about the messageDescriptor issue, I think we can fix it like this:

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

[jeroenvisser101]

Thanks again! 👍

[semoal]

Thanks again! 👍

Thanks to you 🙏