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

Typescript performance regression #2138

Closed
TimoBbz opened this issue Feb 16, 2024 · 11 comments
Closed

Typescript performance regression #2138

TimoBbz opened this issue Feb 16, 2024 · 11 comments
Assignees

Comments

@TimoBbz
Copy link

TimoBbz commented Feb 16, 2024

💥 Regression Report

Checking types of i18next became way more memory-intensive, causing the command yarn tsc to crash out of memory. Running the command NODE_OPTIONS="--max_old_space_size=8192" yarn tsc works but not everyone I work with has that much available RAM.

It seems that the type complexification of the 23rd version caused this regression. Is there a way to restrict the use of the t-function (for example to namespace-prefixed keys only, such as 'ns1:whatever'), and/or a way to generate some type files based on the namespaces to prevent an expensive type-computing?

Thanks in advance!

Last working version

Worked up to version: 22.3.0

Stopped working in version: 23

To Reproduce

Steps to reproduce the behavior:

Having a basic Next project containing 100 namespaces as json files, each with 50 keys.

export const BaseComponent = () => {
  const { t } = useTranslation(['ns1', 'ns2', 'ns3', 'ns4', 'ns5', 'ns6', 'ns7', 'ns8']);
  console.log(t('ns8:lastKey'));
}

Running tsc --generateTrace trace then npx @typescript/analyze-trace trace
It should return the console.log line as very slow.

Expected behavior

It shouldn’t be that slow…

Your Environment

  • runtime version: node v18.17.0
  • i18next version: 23.8.2
  • os: macOS Ventura
  • Everything else is last version
@adrai
Copy link
Member

adrai commented Feb 16, 2024

I'm not a TS expert... not easy to make happy everyone...
Feel free to provide a PR...

@adrai
Copy link
Member

adrai commented Feb 17, 2024

#1883
#2042
#2091

@felixmeziere
Copy link
Contributor

This has been preventing us from updating the library for a year as the types are unusable otherwise :(

@adrai
Copy link
Member

adrai commented Mar 12, 2024

Feel free to provide a PR if you know how this can be improved...
//cc @marcalexiei

@marcalexiei
Copy link
Member

As always, for this kind of issues, a minimal reproduction example would be helpful.

BTW I don’t know if something more can be done to improve memory usage, so if anyone has some ideas a PR would be highly appreciated.

@felixmeziere
Copy link
Contributor

felixmeziere commented Mar 14, 2024

This issue in the typescript repo mentions i18next a lot microsoft/TypeScript#53087 and is the exact problem we have. What explodes between previous versions of i18next and this is the number of type instantiations and memory usage (from `tsc -d --diagnostics).

I've tried to reproduce the conditions of my repo in a new repo but didn't succeed : it could be due to a lot of different things. Only thing I know is that with i18next version pre-v23 tsc runs like a charm but crashes after that...

@felixmeziere
Copy link
Contributor

felixmeziere commented Apr 4, 2024

Hi, we think we have found the culprit for this regression. It is linked with the ternary in FilterKeysByContext that gets evaluated all the time for no reason. We will work with our fix (that moves the ternary up and caches a value) for 2-3 weeks and report back here with a PR if everything works as expected!

@felixmeziere
Copy link
Contributor

Here is what seems to do the trick in the meantime in case someone wants to have a look master...GreenGoTech:i18next:master

@marcalexiei
Copy link
Member

@felixmeziere you can open the PR now if you wish, so we can test you patch against the current types test

@felixmeziere
Copy link
Contributor

I'll open it as draft without comments or anything then

@marcalexiei
Copy link
Member

A fix with the issue of memory usage has been added via #2166 and is available in v23.11.0

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

5 participants