-
-
Notifications
You must be signed in to change notification settings - Fork 629
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
Redesign t
function types
#1911
Conversation
There are a few remaining tasks that I'll be working on in the next few days before merging this PR, that includes:
After merging this PR:
|
6624bde
to
36d6c62
Compare
(this comment is somewhat tangential to this PR, but I think being aware of this can help shape this new typing approach) Hi @pedrodurek, earlier this year, I tried to come up with some type fixes for Maybe we can incorporate some of those ideas into your new typing approach. Also, that PR had some new test cases that might make sense to incorporate here/add later. getFixedT()To me, it seems the types for Adapting your first example from the PR description,
As outlined in #1889 (the bug corresponding to the PR mentioned above), So in fact Lots of TS key suggestionsThese are the same keys that are valid for your To cope with that I came up with the (hack-ish) type The end :)I'm not sure if you can incorporate some of these aspects into this PR or not, but in the long run I think we should address these, so the runtime behavior and the types for |
Perhaps we should make clear what the
Under the hood |
This looks like a fantastic improvement.
|
Thanks for the thourough description of what the PR does @pedrodurek ! I am eager to see this merged, and I am specially interested in typed interpolation params. Until this is merged, I have a quick question on how With const ns1 = {
key1: '',
}
const ns2 = {
key2: '',
}
const resources = {
en: {
ns1,
ns2,
},
} Depending if I call With the first one: const { t } = useTranslation(['ns1', 'ns2'])
t('key1') // no error
t('ns1:key1') // no error
t('key2') // TSC Error: No overload matches
t('ns2:key2') // no error While the later: const { t } = useTranslation<'ns1'|'ns2'>()
t('key1') // no error
t('ns1:key1') // TSC Error: No overload matches
t('key2') // no error
t('ns2:key2') // TSC Error: No overload matches As I understand, using one or the other might be subject to preference or use cases, but curious if there is any reason to use one or the other or if the PR proposal is to deprecated one of these. Thanks! |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
8e3dde3
to
686dfdb
Compare
Can this resolve #1894? |
Benchmark results in Mac M1:Num keys: 11,429 I won’t share the results of the current types because it doesn’t handle these many keys/namespaces. 😅 I’ll share the project that I used for the benchmark once we merge this PR. I do believe that I still can improve the compilation in 30-40%, but I'll work on it in another PR. |
Hey @falsandtru, I'm not sure, but we can work something out once it's merged |
I see |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
48b0f7b
to
ee546e6
Compare
Hey @adrai, it's safe to merge it, I'll update the other PRs once this one is merged.
Done!
Do you mean this?
I'll update the examples with the correct i18n version once the PR is merged. |
fyi: goal is to release a new major version with #1885 and #1945 and also addressing some other stuff: #1884 |
@pedrodurek I mean because of this |
@pedrodurek now all but this is ready to release the major version...
|
seems the compilerOptions needs to be strict: b7075f2#diff-603812bc45f8ec4c682a4be24c9423d325e92c7ea7f10afe1bd84b6c46077164R10 |
stuff to check before releasing this:
|
seems because typeof here is just using string type for the translation values, so there is no info of the interpolation anymore... @pedrodurek a possible workaround:
|
|
Hi guys. This update reintroduced an issue I helped fix in the previous version: Essentially, when the t function receives a default value (either as a string, or as part of the That's the whole point of the default value option. If we're forcing people to always have a value for every key in their main namespace, then this entire option is completely useless, as it will always be overwritten anyway. The same is true by the way for the |
@pedrodurek what do you think? |
addendumMy gut instinct would be to solve this via overloads: /**************************
* T function declaration *
**************************/
export interface TFunction<Ns extends Namespace = _DefaultNamespace, KPrefix = undefined> {
// known keys
<
Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
TOpt extends TOptions,
Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
>(
...args:
| [key: Key | Key[], options?: TOpt & InterpolationMap<Ret>]
| [key: Key | Key[], defaultValue: string, options?: TOpt & InterpolationMap<Ret>]
): TFunctionReturnOptionalDetails<Ret, TOpt>;
// unknown keys
<
TOpt extends TOptions,
Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
>(
...args:
| [key: string | string[], options?: TOpt & InterpolationMap<Ret> & { defaultValue: string }]
| [key: string | string[], defaultValue: string, options?: TOpt & InterpolationMap<Ret>]
): TFunctionReturnOptionalDetails<Ret, TOpt>;
} But you likely had a reason for eliminating them (probably speed?). |
@janpaepke can you try with v23.2.6 ? |
I did and it works. 👍 It let's me set any string as the key, when a default value is provided and complains if there's not. There's another thing I noticed, but I am not entirely sure if it's related to my environment (IDE/ts version). Key-Autocompletion for the t function doesn't seem to work for me any more. Should I create a separate issue for that? |
@janpaepke intellisense is tracked here: microsoft/TypeScript#54708 (probably fixed in next ts version and/or ide version) |
@adrai thanks a lot! |
In a React Component, when I'm loading a namespace
The TS error says Should you update this documentation of loading namespace in react-i18next ? Thanks in advance for your answer |
@MiryksV please provide a complete but minimal reproducible example repository or similar... |
I couldn't reproduce on my sandbox, I'll take a deeper look in my project. Sry to bothering you for nothing. |
I just adopted this, and the typescript compiler needs about 8.5GB ram now in order to type check our code. We use the interface generation, because we have our translations in .json files. Without the generated interfaces included, typescript needs about ~3s to run. If you have any ideas on why it might take so long, I would really appreciate it! Edit: after getting rid of all TS errors, it went down to 3s runtime, and about 400Mb ram. |
Seems like, once I had a no-error run the tsc cache really speeds up the compiler. |
Hey, before i can use generic type to specify which type would return How can i do the same with new version, when passing third param ts complain that 'Type string[] does not satisfy the constraint string'? |
I have the same issue as @reckter with very long tsc run times because of this, any way to solve the issue ? |
Description
The existing implementation of the type for the
t
function is characterized by being very complex, slow, hard to maintain, and not scalable, which are also hampered by various limitations.Our redesign endeavours to enhance the approach to parsing and inferring keys for the
t
function. Instead of performing a recursive examination of each key-value pair inresources
associated with specific namespace(s) each time thet
function is invoked, we generate a comprehensive set of keys from all namespaces just once. Then, we parse and filter the keys based on the designated namespace and key prefix. The outcome of this redesign is that the types have been greatly simplified, and are faster, smarter, more scalable, and are easier to maintain. This upgrade will allow us to mitigate numerous issues raised by devs.Features
t
function will infer and accept the keys for the first namespace. So this pattern will be accepted now:t
function will now infer and accept the keys for the main namespace (i18next):returnObjects
) that will infer fewer keys if set tofalse
, and all keys and sub-keys if set totrue
. If the optionreturnObjects
fromt
function is set totrue
, it'll work the same way:t
function will now infer interpolation values, but it'll only work if the translation files (resources) are placed in a ts file and usingas const
, JSON files don't supportas const
to convert objects to be type literals.This will close: Type errors when doing interpolation in react-typescript/simple react-i18next#1598, Typesafe Interpolation Variables #1897 and Typescript safer interpolation react-i18next#1547
Fixes
This'll close: i18next/react-i18next#1422, i18next/react-i18next#1571 and #1886.
And it'll probably close: i18next/react-i18next#1600, i18next/react-i18next#1601, i18next/react-i18next#1608, #1901 and #1883.
Many more...
Breaking changes
All breaking changes described below are minor ones:
returnObjects
set astrue
by default will also have to set the same option in theCustomTypeOptions
type. Otherwise, only complete keys will be allowed (key1.key2.key3...
).StringMap
to$Dictionary
, and we'll no longer export it.$Dictionary
is an internal helper, and there is no reason to expose it. If needed, you can create a copy and reuse it in your project.ns
property fromInterpolationOptions
type will be constrained toNamespace
rather thanstring
orreadonly string[]
.KeysWithSeparator
type toJoinKeys
, and it will no longer be exposed.TFuncKey
type toParseKeys
, and it will no longer be exposed.NormalizeByTypeOptions
type.DefaultTFuncReturnWithObject
type toDefaultTReturn
. It will acceptTOptions
as generic constraint and will no longer be exposed.DefaultTFuncReturn
type in favour ofDefaultTReturn
.NormalizeReturn
type.Hopefully I didn't forget anything 😅 .
Benchmark results
Num keys: 11,429
Num namespaces: 55
Compilation time with the types: ~6.27s
Compilation time without the types: ~1.49s
VSCode autocomplete keys (without cache): ~2.477s
Limitations
ns
option fromt
function, all namespaces passed are considered default.We'll be able to tackle this issue in typescript 5 with the new
const
Type Parameters. Alternatively, you can convert the array to literal values withas const
:as const
keyword. We'll support it once this issue is addressed: import ConstJson from './config.json' as const; microsoft/TypeScript#32063Checklist
npm run test
Checklist (for documentation change)