CustomTypeOptions pollute and break all other instances

[falsandtru]

Described at #1971 (comment)

[adrai]

Ahh you mean other i18next instances...

Yes, typed translations only work for 1 i18next instance now.

I don't know if it could be ever possible to have typesafe translations for multiple i18next instances in the same project... 🤷‍♂️

[falsandtru]

Yes. This is same as global variable pollution. CustomTypeOptions must be parameterized and capsuled but actually defined as a global type affecting all instances.

[adrai]

@henrikvolmer may I ask your opinion regarding this?

[henrikvolmer]

Can you please provide a full example in codesandbox.io.

[adrai]

@henrikvolmer he means, something like this does not work... https://github.com/adrai/ts-i18next-multi-instances/blob/main/index.ts
and I suspect this will never work

[falsandtru]

Never work with appropriate designs. Handling a local state via/as a global state is a very bad approach/design. That is an obvious anti-pattern and many problems actually arise easily. That has to be rescinded for the time being. Remember that overly complex types usually prevent basic use. Advanced type safety is not worth sacrificing basic type safety. Advanced safety devoid of basic safety is fake.

[falsandtru]

A correct and complete fix is difficult or impossible, and it would take a long time. Can you remove the wrong type definitions for the time being? Otherwise, it would remain severely broken for a long time. In addition, performance is very likely to deteriorate. If it does not work in actual development, it is worthless. Make it fast.

[adrai]

A correct and complete fix is difficult or impossible, and it would take a long time. Can you remove the wrong type definitions for the time being? Otherwise, it would remain severely broken for a long time. In addition, performance is very likely to deteriorate. If it does not work in actual development, it is worthless. Make it fast.

@pedrodurek do you know what @falsandtru means?

[adrai]

fyi: based on the feedback we are receiving for v23 it seems much better than old versions 🤷‍♂️

i18next/react-i18next#1608 (comment)

[falsandtru]

Users in TS would be few or not using CustomTypeOptions. Well, at least the issues of performance problems are reported: #1972, #1956, #1921, #1883. Fixing it correctly is not enough. Correct deep types will make it even worse. It will cause even more users to not be able to use your types and product 🤷‍♂️

[falsandtru]

Note that the actual issues, including duplicates, are much higher. Of course, potential numbers are not countable.

[henrikvolmer]

May I ask you to help us to fix the problem instead of just complaining about how bad the update (in your eyes) is? If the new version is actually not working for you, just downgrade it to the older version in your project. Just asking for a complete revert of a major version is not helping at all.

[falsandtru]

It's a futile work. You don't understand that too bad to fix all! Understand how it is bad before doing something.

[adrai]

@Malien may I ask also your opinion/feedback regarding this etc...?

[falsandtru]

Maintainer's answer:

#1971 (comment)

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.

#1971 (comment)

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

#1971 (comment)

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

Given the above, CustomTypeOptions will be a trap for a large growing product on TypeScript as many performance problems are reported.

[Malien]

I'll still chime in my two cents tho.

First of all, performance issues are still at large. I'm thinking moving towards integrating more things into the bespoke build-system, in order to ease typescript type checking, among other things it already does to translations. But that's not exactly npm installable. Although in any case, I think build system shenanigans are required to achieve optimal performance on any client-side project utilizing i18next, so maybe having it as a public package is a worthwhile goal.

As in global type pollution, well it is definitely a thing. Although I doubt there are extensive uses of i18n.createInstance out there in the wild, it is still worth addressing.

Untype i18n.createInstance

One way would be to have i18n.createInstance return an "untyped" i18n instance. Meaning it doesn't do the whole CustomTypeOptions resolution, and default to the classic untyped semantics. I does require having two I18n types, and retyping existing functionality tho. This is nice and simple "eye-patch" solution.

Passing types during instance creation (Parametrize I18n<T> type)

Personally, I'd rather opt-out from the current approach of initializing global, in favour of creating an instance. Current approach would also mean, that changing import order, could result in use of uninitialized global instance. During instance creation we should be able to pass in types and have them be associated to the created instance:

import { createInstance } from "i18next"
import { createHelpers } from "react-i18next"
import type Resources from "somewhere-there-are-translation-typings"

const i18n = createInstance({
    fallbackLng: 'en',
    debug: false,
    returnObjects: true
  }) // I18n<{ returnObjects: true }>
  .use(somePlugin) // Possibly modified I18n type by plugin
  .resourceType<Resources>() // I18n<{ returnObjects: true, resources: Resources }>

export default i18n
export const { useTranslation, Trans } = createHelpers(i18n)

The API is just a brain-fart. This is a hella of a breaking change, but it does address multiple instances usecase as well. There are many variations of this design possible, such as:

createInstance<Resources>().use(plugin).init(opts)

which I think follows the current initialization flow more closely.

createHelpers(i18n) might fix-in-place use of the specified i18n instance, meaning <I18NextProvider /> can't affect these. This may hinder testing tho.

Or alternatively it might just yoink the options type from existing instance, and set it as the "default". This would mean, that having <I18NextProvider /> in the render tree can break typesafety:

import { createInstance } from "i18next"
import { createHelpers } from "react-i18next"
import ReactDOM from "react-dom/client"

const i18n = createInstance<{ ns: { foo: string } }>().init({
  resources: {
    en: {
      ns: { foo: "Faz" }
    }
  }
})
const { useTranslation } = createHelpers(i18n)

const rogueI18n = createInstance<{ ns: { bar: string } }>().init({
  resources: {
    en: {
      ns: { bar: "Baz" }
    }
  }
})

function MyComponent() {
  const { t } = useTranslation("ns")
  return t("foo") // This thinks it is fine
}

ReactDOM.createRoot(...).render(
  <I18NextProvider i18n={rougueI18n}>
    <MyComponents /> {/* Oopsie daisy */}
  </I18NextProvider>
)

To be honest. There are SOOO MANY ways to break typesafety of i18next, that this might not be an issue. (For e.g. you can have incomplete translation files in some of the languages, but not others)

Once again, proposed API is just a brain-fart, and here just to bring up possible solutions.

[falsandtru]

working as expected/designed label is just displaying that they cannot understand their own misdesign and they designed their own misdesign 😉