i18next / i18next-http-backend

i18next-http-backend is a backend layer for i18next using in Node.js, in the browser and for Deno.
MIT License
452 stars 70 forks source link

Usage in Nuxt plugin : detection of a memory leak #150

Closed PReynaud closed 1 month ago

PReynaud commented 1 month ago

🐛 Bug Report

Hello, I'm working for my company on a Nuxt 3 stack. We've been using i18next for several months to manage our translations and it worked well. However, our bundles were getting too large, so we decided to find a solution and try this library. We added it, did the configuration and it worked well :) But after deploying, we quickly noticed a memory leak.

Solution

After digging, I found that the use of setInterval in the initialization of the Backend class was problematic and was the root cause of our issue. I patched it using pnpm to replace it with a setTimeout to 0. We are now back in production without issues.

To Reproduce

Minimal reproduction here: https://stackblitz.com/~/github.com/PReynaud/i18next-http-backend-memory-leak-reproduction?file=plugins/i18next.ts

The main code is in a Nuxt plugin and looks like this :

    const i18next = createInstance();

    const i18nConfiguration: InitOptions = {
      lng: "fr",
      load: "languageOnly",
      debug: true,
      supportedLngs: ["fr", "en"],
      backend: {
        loadPath: "/public/locales/locales.{{lng}}.json",
        request: async (_: any, url: string, __: any, callback: any) => {
          try {
            const response = await useFetch(url);
            if (response) {
              callback(null, { status: 200, data: response });
            } else {
              callback(
                new Error("Error while fetching current translation file")
              );
            }
          } catch (e) {
            callback(e);
          }
        },
      },
      postProcess: [],
    };

    i18next.use(Backend);

    await i18next
      // Init i18next. For all options read: https://www.i18next.com/overview/configuration-options
      .init(i18nConfiguration);

    (nuxtApp.vueApp as App).use(I18NextVue, { i18next });

Your Environment

adrai commented 1 month ago

I assume you have a serverless backend? For a normal server backend the setInterval is important... that's why there is this check: So if you don't need that, you can simply pass reloadInterval: false Have you tried that?

image

Beside that, are you sure that is causing the memory issue? Can you share the code you changed from setInterval to setTimeout?

While inspecting I could not find the memory leak with reloadInterval: false

PReynaud commented 1 month ago

Hi, thank you for the quick answer. We use a normal server for that, but we do not need to reload the translations at runtime.

reloadInterval to false seems to work indeed. I was pretty sure to have tested that but I surely have messed something in my config at this moment (the render on server side was not working). So I'm sorry for the false report.

In any case, if this option is not set to false, there is a memory leak. It's probably something worth mentioning in the documentation?

adrai commented 1 month ago

Why do you say it is a memory leak? Can you show me why you say it is a memory leak? The setInterval just reloads all translations for a given interval... this way it keeps the translations up to date on server side... Usually you have just 1 i18next instance and it reloads the translations.... that's all...

adrai commented 1 month ago

since I'm not a Vue or nuxt user, maybe @kkuegler can help to clarify?

PReynaud commented 1 month ago

If I'm not mistaken, the setInterval is never cleared. In our use case, we run a new instance for each user (to avoid sharing context between the users) and so, with the reload mecanism, the memory will slowly increase.
But of course reloadInterval: false is necessary for this and I thank you for pointing me to that.

adrai commented 1 month ago

Creating a new instance for each new user (or http request) is not a very good idea... that is probably the root problem for that memory leak... why not cloning the i18next instance...? like done here: https://github.com/i18next/i18next-http-middleware/blob/master/lib/index.js#L46

PReynaud commented 1 month ago

We had problems in the past with Nuxt and shared context between users, so we ended up with this solution and it worked well. I'll have a look to the cloning method, it could fit better indeed 👍