Add test

[falsandtru]

Add a failed test requested in #1894.

[adrai]

If you know of how to best address this in the types, feel free to extend this PR...
I just know that @pedrodurek wanted to eliminate the overloads to improve the performance... 🤷‍♂️

[adrai]

btw: fixing this will probably also help for: i18next/react-i18next#1571

[falsandtru]

Unfortunately I don't know of any solution to this other than overloads. And I don't know react. BTW, performance problems seem to be increased after eliminating the overloads: #1972, #1956, #1921, #1883. I don't know but do overloads generally worsen performance?

[adrai]

Ok, than try with the overloads... at least we have a possible solution...

[falsandtru]

Sounds good. I want to use the latest version.

[adrai]

Probably something like this?

export interface TFunction<Ns extends Namespace = _DefaultNamespace, KPrefix = undefined> {
  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions & { returnObjects: true },
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    options: TOpt & InterpolationMap<Ret> & { returnObjects: true },
  ): TFunctionReturnOptionalDetails<Ret, TOpt>;

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions & { returnDetails: true},
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    options: TOpt & InterpolationMap<Ret> & { returnDetails: true},
  ): TFunctionReturnOptionalDetails<Ret, TOpt>;

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions,
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    options?: TOpt & InterpolationMap<Ret>,
  ): TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>;

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions & { returnObjects: true },
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    defaultValue: string,
    options: TOpt & InterpolationMap<Ret> & { returnObjects: true },
  ): TFunctionReturnOptionalDetails<Ret, TOpt>;

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions & { returnDetails: true },
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    defaultValue: string,
    options: TOpt & InterpolationMap<Ret> & { returnDetails: true },
  ): TFunctionReturnOptionalDetails<Ret, TOpt>;

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions,
    Ret extends TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>,
  >(
    key: Key | Key[],
    defaultValue: string,
    options?: TOpt & InterpolationMap<Ret>,
  ): TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>;
}

[falsandtru]

Probably Ret should be removed.

  <
    Key extends ParseKeys<Ns, TOpt, KPrefix> | TemplateStringsArray,
    TOpt extends TOptions & { returnObjects: true },
  >(
    key: Key | Key[],
    options: TOpt & { returnObjects: true },
  ): TFunctionReturnOptionalDetails<TFunctionReturn<Ns, AppendKeyPrefix<Key, KPrefix>, TOpt>, TOpt>;

This breaks some tests but the current definition is originally wrong. The next test is valid.

  const o: object = t('friend', { returnObjects: true });

But the actual type without type annotations is DefaultTReturn<{ returnObjects: true; }> not assignable to object.

  const o = t('friend', { returnObjects: true });

Current definitions essentially just fit the type to annotations, as the next.

type DefaultTReturn<TOpt extends TOptions> =
  | string
  | TReturnOptionalObjects<TOpt>
  | TReturnOptionalNull;
...
    Ret extends DefaultTReturn<TOpt>,

[adrai]

Need the advice of @pedrodurek here....

[falsandtru]

Added the tests. Tests are confusing type casts and type annotations. They must use true type casts.

[falsandtru]

Essential difference:

// Expected
declare function f(): string | object;
const a = f() as object; // Correct usage of type cast

// Actual
declare function g<a extends string | object>(): a; // Wrong usage of type parameter
const b: object = g(); // Wrong usage of type annotation

[adrai]

I tried with some overloads, but I really don't know if this is the way to go... I would really appreciate @pedrodurek help here

[falsandtru]

Fixed except lints.

[adrai]

Does not work anymore:
https://github.com/i18next/i18next/pull/1971/files#diff-62b6742de09d87fba9495aac7a5ffb538e1d3c69af885232d60adb333b676151R23

[falsandtru]

That error is correct. The return type of TFunction is undetermined to be string yet.

[adrai]

That error is correct. The return type of TFunction is undetermined to be string yet.

so can you update the test?

[falsandtru]

Ah no, another problem. This is a correct behavior of overloads.

// Type '() => string' is not assignable to type '{ (): string; (): object; }'.
//  Type 'string' is not assignable to type 'object'.ts(2322)
const f:{
  (): string;
  (): object;
} = () => '';

You have to use another interface.

[falsandtru]

However, t('translationKey').trim() will break anyway.

[falsandtru]

If you want to implement TFunction, you have to implement polymorphism. () => string is a partial type of TFunction, not a complete type of TFunction.

[adrai]

Since I'm not a TS user, we have to wait for @pedrodurek's comment.

[falsandtru]

Well, keeping all tests is very difficult or impossible because tests are based on many broken types.

[falsandtru]

Functions and type safety in mock tests are totally broken...

[falsandtru]

Current custom type definition via CustomTypeOptions is not extensible for new instances. It would be better that providing official advanced functions for default instance and deep typing. Basic APIs should be type safe and light weight.

[falsandtru]

Current definitions have too many broken types. You should restart from the previous definitions (v22.4.4). Probably that is better than this pr. However, tests should be merged.

[falsandtru]

Reverted. Update types and tests again. I can fix it before merging.

[adrai]

<v23.x.x had a lot of issues for typed translations usage... but like said, I'm not a TS expert...

[falsandtru]

Current broken types are shipped since 22.4.5. Some of those seem to be caused by the current broken types. Anyway the cost of fixing them will be fewer than the cost of fixing the current broken types. The correct types are closer to the previous one than the current one, and the related issues are few.

[falsandtru]

Additionally, do you understand that CustomTypeOptions pollute and break all other instances? Current deep type safety is fake. In fact, that breaks type safety of all other instances. See added tests.

function anotherInstance() {
  // CustomTypeOptions pollute and break all other instances.
  const i = i18next.createInstance({
    lng: 'en',
    resources: {
      en: {
        translation: {
          'a': 'b',
        },
      },
    },
  });
  i.init();

  const t = i.t;
  t('a'); // Correct but error
  t('common').foo // Wrong but no error
}

[falsandtru]

Fundamental misdesign. Too many and wide faults to fix all.

[pedrodurek]

Hey @falsandtru, just wanted to clarify that runtime and compile-time are two different things. Typescript checks types during compile-time, so it can't infer types during runtime.

If your resources change dynamically, you have two options:

1. Set your resources from CustomTypeOptions with all possible options. However, since the resources may change over time, the type safety can no longer be guaranteed.
2. Don't set resources from CustomTypeOptions, which means keys will be assigned as string | string[] | TemplateStringsArray. But, this means that the t function won't have type-safe and auto-completion features.

We understand that the type enhancements here may not be suitable for everyone. That's why defining the resources type is optional.

[falsandtru]

In short, setting the resource type of CustomTypeOptions is incompatible with another instances and options? It may be ok if setting it doesn't cause the performance problems. All the performance problems are caused by setting it?

[pedrodurek]

@falsandtru, yes, that's correct. When you set resources under CustomTypeOptions, we map over all object keys within resources to infer the t function's keys and its exact return type. However, depending on the project size or your configuration, it might not be suitable for you. We added this information in the document https://www.i18next.com/overview/typescript.

[falsandtru]

Can you explain why you are trying to get us to use a feature that will break down as the project grows?

[pedrodurek]

Depending on your project's size, not even typescript without the i18next lib will work as expected. Talking from experience 😉

Anyway, it still performs really well, you can take a look at the benchmark results here: #1911

If you're not happy with the results here or if something doesn't fit your personal needs, you can always use Module Augmentation or patch-package to write your own types. Alternatively, you can create your own solution from scratch.