Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow unknown keys, when a defaultValue is provided #1867

Closed
janpaepke opened this issue Nov 16, 2022 · 9 comments
Closed

Allow unknown keys, when a defaultValue is provided #1867

janpaepke opened this issue Nov 16, 2022 · 9 comments
Assignees

Comments

@janpaepke
Copy link

janpaepke commented Nov 16, 2022

💥 Regression Report

This isn't really a functional regression, more like a convenience regression, it only concerns the new Typescript enhancements.
When using the t function it is now impossible to use with keys that don't exist in the default namespace, even when a default value is provided, which it could fall back to.

This forces me to always add every key to my default namespace's resource file.
Usually in our workflow we only add translations inline using the defaultValue, when developing a new feature.
This is because things often change.
Only once something is production ready, we run i18next-parser to extract all keys and add translations.

Proposal

I propose to add additional overloads for when a default value is provided (one for t('key', 'defaultValue'), one for t('key', { defaultValue: 'value' }), where the key can be any string.

Last working version

Worked up to version: 21

Stopped working in version: 22

To Reproduce

Steps to reproduce the behavior:

t('unkown.key', 'default translation')
// ⬆ No overload matches this call.
//  The last overload gave the following error.
//    Argument of type '"unkown.key"' is not assignable to parameter of type 'Normalize<{ ... }>  | TemplateStringsArray ...`

Your Environment

  • runtime version: i.e. node v16
  • i18next version: i.e. 22.0.5
  • os: Mac
@adrai
Copy link
Member

adrai commented Nov 16, 2022

Would you like to send a Pull Request to address this? Remember to add unit tests.

@janpaepke
Copy link
Author

janpaepke commented Nov 16, 2022

I'll put it on my todo list. It's long. 😉

@janpaepke
Copy link
Author

janpaepke commented Nov 17, 2022

So I had some time and adding the overloads was actually pretty straight forward.

here they are:

  <
    TDefaultResult extends DefaultTFuncReturn = string,
    TInterpolationMap extends object = StringMap,
  >(
    key: string | string[],
    options: TOptions<TInterpolationMap> & { defaultValue: string },
  ): TFuncReturn<N, string, TDefaultResult, TKPrefix>;
  <
    TDefaultResult extends DefaultTFuncReturn = string,
    TInterpolationMap extends object = StringMap,
  >(
    key: string | string[],
    defaultValue: string,
    options?: TOptions<TInterpolationMap> | string,
  ): TFuncReturn<N, string, TDefaultResult, TKPrefix>;

I tested them and they work as expected.
They even still prefer the known keys so key autocomplete still works, when a default value is provided.

I might have to add a few more since the return value is different, when returnDetails and/or returnObjects is true.

I could create a pr, but since this is a types-only change, I failed to find the appropriate testing suite (was looking for tsd).

Can you point me in the right direction?

@janpaepke
Copy link
Author

A quick check of those didn't help.

In typescript/t.test.ts I can add any key and npm run test:typescript doesn't fail.

The file typescript/custom-types/t.test.ts seems to be specifically made to test the "known keys" feature.
However running npm run test:typescript:custom-types also doesn't complain if I change one of the keys.
It even complains about the @ts-expect-error directives: TypeScript@next compile error: Unused '@ts-expect-error' directive..

So I'm guessing this might be a configuration issue?

Pls advise.

@pedrodurek
Copy link
Member

Hey @janpaepke, sorry for the delayed response. Actually, I'm trying to get rid of the some function overloads due to some limitations and to speed up the compilation time. I'll support what you want, just give me a few more days.

@janpaepke
Copy link
Author

Ok sure! Let me know if I can help in any way!

Also: no need to apologise 😊.

@adrai
Copy link
Member

adrai commented Dec 11, 2022

@janpaepke try v22.4.5 in the meantime... it might be some performance changes will need to be done but in the meantime I added also some other t function overloads....

@janpaepke
Copy link
Author

works as expected. I guess this concludes this issue? Should we close it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants