return value of t can be string or object, array

Coverage remained the same at 88.413% when pulling 8428b50 on VincentLanglet:patch-1 into f033840 on i18next:master.

Thanks @jamuhl ; I changed the type

I'll be happy to have the review of the previous author : @tkow

No problem for me. It's good improvement, thanks.

@jamuhl This can be a patch release. Not urgent, can be included in any next batch release.

This turned out to have been surprisingly breaking. I'm just not sure if it's a good or a bad break.

Every single use of t where an object result is not expected is now a type error.

I wonder if there is a way to declare, somewhere, what kind of result a specific key should have. Because of how typescript generic arguments work, the TResult is very inconvenient to declare (you're forced to pass the key name twice and then the type of TValue). Finding a way to that could make TValue itself safer.

This could be made non-breaking by changing the default to be the same as the old value. It's still not perfect, though. Looking at how the old type was inferred it seems to actually prefer a contextual type if one is available, only ever falling back to any if there is no context. With the new default it ignores any contextual type and just errors everywhere.

Getting types right for i18next will probably require adding generics in a bunch of other places, including the interfaces and the components from react-i18next and other bindings, not just to the function inside WithT. As it stands, all of those generic arguments are hidden anys / hidden as casts. It's only the defaults and the extends that are making them not behave directly like that.

Why these arguments are "hidden anys" (well, "hidden whatever the type of the extends are"): they are never related to anything. As in, they are never used to create a relation between two types, or between an input and an output.

Let's take this t function, formatted for readability:

t<
  TKeys extends string = string,
  TValues extends object = object,
  TResult extends string | object | Array<string | object> =
    | string
    | object
    | Array<string | object>
>(
  key: TKeys | TKeys[],
  options?: TranslationOptions<TValues>
): TResult

The type of key is exactly the same as string | string[]. The type of options is exactly the same as TranslationOptions<object> | undefined. And the type of TResult is exactly the same as its default.

Having a TKeys more specific than string serves no purpose to the signature itself. Because TKeys is never used to relate the key to, say, the options (perhaps by mapping through a lookup table given as generic argument to WithT?), it will just be inferred to whatever it is that you pass to it, and will still accept any string you want.

The same goes for TValues itself, and TResult. They are only "useful" if you explicitly write the generic argument... which means they're about the same as as casts.

Also, because these generics are in the function itself instead of passed through a chain derived all the way from the i18n instance, they are still meaningless in the context of your translations.

This is actually something that is caught by a custom tslint rule they have running in DefinitelyTyped but is not in tslint core or is otherwise easy to use from outside DefinitelyTyped.

Is this an argument for moving types to DT? An argument for DT to make their rules more widely available? Who knows, they might port them over to eslint given how it's in their roadmap now.

Contrast this with the way redux implemented its A generic argument. All A must extend Action, but the acceptable A is derived from the type of the reducer you give to createStore, and the type of dispatch is derived from the A that was inferred from your reducer. A is not itself a generic argument in dispatch's function signature, which is what is happening here.

To clarify, the dispatch signature is generic, but the generic constrained to A instead of being A itself. It's the A that was derived by createStore. The generic argument in dispatch also actually is used more than once (the type of the output is the same as the type of the input). If it did not also show up as the return type, the signature could have been just a non-generic (action: A): void.

See the master where the ts usage tests are broken apart. @tkow generates TKeys for usage. This is probably better filed as an open issue with tagged participants. Commenting on a merged PR is not bound to result in a response or change.

Every single use of t where an object result is not expected is now a type error.

BTW @Jessidhia I think I have encountered some of what you are experiencing while refactoring and understand the cause one of your issues - a mismatch between TranslateFunction and WithT. I'm working on it here #1180