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

Memoized string-only translation does not update on useLingui re-render #1690

Closed
niksauer opened this issue Jun 5, 2023 · 15 comments · Fixed by #1701
Closed

Memoized string-only translation does not update on useLingui re-render #1690

niksauer opened this issue Jun 5, 2023 · 15 comments · Fixed by #1701

Comments

@niksauer
Copy link

niksauer commented Jun 5, 2023

Describe the bug
In v4, to ensure that messages tagged with the t macro get updated when the locale changes, we must subscribe components to the useLingui hook. This works reliably in the component's return-to-render. However, if parts of the render are memoized using a hook such as useMemo, the value will not always be updated. Always, because this seems to only happen after switching the locale at least twice. The first change is registered. Yet, when i18n.locale is added to the hook's dependencies, values are reliably re-computed.

To Reproduce

// LocaleProvider.tsx

  // When the language changes, load and activate its message catalog.
  useEffect(() => {
    async function activate(language: SupportedLanguage): Promise<void> {
      const { messages } = await import(`../../assets/locales/${language}/messages`);

      i18n.load(language, messages);
      i18n.activate(language);

      setCachedLanguages((cachedLanguages) => {
        return { ...cachedLanguages, [language]: true };
      });

      setShowLoadingSpinner(false);
    }

    activate(activeLanguage);
  }, [activeLanguage]);

[...]

  return (
    <LocaleContext.Provider value={context}>
      <I18nProvider i18n={i18n} {...props} />
    </LocaleContext.Provider>
  );

Does not work reliably:

const translatedMessage = useMemo(() => i18n._(msg`Unverified`), [i18n]);
console.log(translatedMessage); // Unverified

Works reliably:

const translatedMessage = useMemo(() => i18n._(msg`Unverified`), [i18n, i18n.locale]);
console.log(translatedMessage); // Sin verificar

Additional context
Add any other context about the problem here.

  • jsLingui version: 4.2.0
  • Babel version: @babel/core@7.21.5 (via @lingui/cli@4.2.0)
  • Create React App: 5.0.1
@timofei-iatsenko
Copy link
Collaborator

It's just a mistake in the docs. Indeed, you should add i18n.locale to the deps array, because i18n itself is not changing.

@timofei-iatsenko
Copy link
Collaborator

Wait, did you try to depend on i18n instance returned from useLingui hook, as it stated in docs ?

@niksauer
Copy link
Author

niksauer commented Jun 13, 2023

Thanks for the clarification. Not updating due to the stable identity makes sense. Unfortunately, this means that many projects will get an eslint[react-hooks/exhaustive-deps] warning because i18.locale needs to be added even though it's not directly used. Would it be possible to expose a helper on the useLingui hook that does string-only translations and can be used as the dependency itself? Something like:

const { i18n } = useLingui();
const { t } = i18n;

const welcome = useMemo(() => {
  return t`welcomeMessage`;
}, [t]);

Or is thing something that we should rather do as a custom hook in our project?

@timofei-iatsenko
Copy link
Collaborator

You will not able to do this as custom hook, because t is virtual entity. It's get transformed in compile time.

Actually using global t is discouraged, it's better to use t with instance of local i18n, this way you will not have problems with exhaustive-deps

const { i18n } = useLingui();

const welcome = useMemo(() => {
  return t(i18n)`welcomeMessage`;
}, [i18n]);

@niksauer
Copy link
Author

This still doesn't work except when adding i18n.locale to the deps:

import { t } from '@lingui/macro';
import { useLingui } from '@lingui/react';

const Component = () => {
  const { i18n } = useLingui();

  const welcome = useMemo(() => {
    return t(i18n)`welcomeMessage`;
  }, [i18n, i18n.locale]);

  return welcome;
};

Am I missing something?

@timofei-iatsenko
Copy link
Collaborator

@vonovak You added this section in docs https://lingui.dev/tutorials/react-patterns#memoization-pitfall could you elaborate a bit here? Does i18n instance returned from useLingui(); get renewed when something is changed?

@vonovak
Copy link
Collaborator

vonovak commented Jun 14, 2023

hello, my mistake, sorry. I wrote the docs badly. You can depend on i18n.locale, or on the context object itself. The reference to i18n object does not change.

@niksauer
Copy link
Author

What is the context object?

@vonovak
Copy link
Collaborator

vonovak commented Jun 14, 2023

What is the context object?

const contextObject = useLingui()

sorry I didn't make it clear. This will be more robust because it will react to updates to message catalogs as well.

@tingyuzeng18
Copy link

tingyuzeng18 commented Jun 14, 2023

@vonovak For me depending on the i18n in the context will not trigger the lazy translation to be changed once the locale is changed. It only works if I add i18n.locale to the dependency array as stated in this reply. But indeed it breaks eslint rules: "exhaustive-deps".

@vonovak
Copy link
Collaborator

vonovak commented Jun 15, 2023

hello, I updated the docs here https://github.com/lingui/js-lingui/pull/1701/files

@tingyuzeng18
Copy link

tingyuzeng18 commented Jun 15, 2023

@vonovak Sorry I was not clear: I used the i18n in the context returned by useLingui, but the lazy translation is only updated if i18n.locale is in the dependency array. I see in the PR only i18n is in the dependency array, which did not work for me.

@niksauer
Copy link
Author

niksauer commented Jun 15, 2023

Both versions below do indeed work, though I wish there was a shorter way to do a memoized string-only translation:

import { t } from '@lingui/macro';
import { useLingui } from '@lingui/react';

const Component = () => {
  const lingui = useLingui();

  // Works
  const version1 = useMemo(() => {
    return t(lingui.i18n)`welcomeMessage`;
  }, [lingui]);

  // Works
  const version2 = useMemo(() => {
    return lingui.i18n.t("welcomeMessage");
  }, [lingui]);

  [...]
};

@vonovak
Copy link
Collaborator

vonovak commented Jun 15, 2023

I'm away for a few days now but believe there is a way to make this less verbose: we can expose the 't' function from the context in such a way that it will change reference as we need.
Then, the memo would depend on just 't' and not 'i18n'.
I will take a look at this in two weeks or so. There might be some downsides that I haven't considered.

@vonovak
Copy link
Collaborator

vonovak commented Jul 3, 2023

@niksauer @tingyuzeng18 @thekip I opened the PR here #1721; feedback is welcome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants