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

Improve React syntax with useLingui #1852

Closed
dan-dr opened this issue Feb 19, 2024 · 13 comments · Fixed by #1859
Closed

Improve React syntax with useLingui #1852

dan-dr opened this issue Feb 19, 2024 · 13 comments · Fixed by #1859

Comments

@dan-dr
Copy link
Contributor

dan-dr commented Feb 19, 2024

Is your feature request related to a problem? Please describe.
I'm implementing lingui for my company's product, with DX being among the top priority.
The syntax when using the lingui hook is quite clunky.

First, there are 2 ways to translate (correct me if i'm wrong):

import { t, msg } from '@lingui/macro';

// react component
() => {
  const { i18n, _ } = useLingui();
  const one = _(msg`First way to translate`);
  const two = t(i18n)`Second way to translate`;
}

Second, each of these ways are somewhat "special" syntax with 2 parts each (_ and msg, t and i18n).

I find the DX and readability sub-par with the high quality of this library and it's tooling.

Describe proposed solution
Basically I'd like to do something like:

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

// react component
...
  const { i18n, _, t } = useLingui();
  const justOneWay = t`Very Cool`;
...

I'm guessing the macro should transform this to:

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

const { i18n, _ } = useLingui();
const justOneWay = _({ ...message descriptor... });

Not sure exactly about the naming or the macro API. maybe it's better to separate the hooks

const { t } = useTranslate()
t`String`;
 becomes
const { _ } = useLingui();
_({...descriptor});

but then you probably need to account for other use of useLingui and merge the declaration, which isn't impossible.

Describe alternatives you've considered
I tried to play around with a custom hook to wrap, but I don't think it's possible with the macro implementation.
neither of these hacks are possible (and probably for a good reason:

  const t = React.useCallback(
    (literals: TemplateStringsArray, ...placeholders: any[]) => {
      return _(defineMessage(literals, placeholders));
      nor
     return t(i18n)(literals, placeholders);
    },
    [.. dependencies],
  );

Additional context
I'm not familiar with macro implementation, else I'd give it a whirl myself...

@timofei-iatsenko
Copy link
Collaborator

The current design here for a reason. Indeed, the design proposed by you is better and I even investigated to possibility to do something like that before. However, there are technical restrictions on the babel-macro side and also this kind of transpilation opens a wide variety of usages user's could do with these functions which we could not know ahead of time and it would be a big maintenance burden to handle all possible cases in transpilation.

The current flow is much simpler, we take 1 existing node and replacing to another one. To achieve what you're proposing you need replace multiple related to each other nodes and traverse AST back and forth which is waay more complicated process.

@wentokay
Copy link

wentokay commented Feb 20, 2024

I had similar thoughts about this today and while I was trying the different suggested approaches, I noticed that this

const Component = () => {
  useLingui();
  return <div>{t`Always updates after switching locales`}</div>;
}

effectively behaves the same way as

const Component = () => {
  const { i18n } = useLingui();
  return <div>{t(i18n)`Always updates after switching locales`}</div>
}

I assume this is an unexpected/unsupported behavior?

I haven't dug too deep into the lingui source yet but I assume it's because useLingui() is updating the i18n reference used by the macro below it with what it gets from React Context?

I don't think I'll use that approach in the long term, but we just migrated from i18next and with 300+ files to update it might be a quick and useful temporary fix while figuring out a better long term strategy.

I'm currently trying to see if there's any way to guarantee that i18n is a global singleton shared across our monorepo, rather than needing to inject it in every t``.

@timofei-iatsenko
Copy link
Collaborator

const Component = () => {
  useLingui();
  return <div>{t`Always updates after switching locales`}</div>;
}

This is supported and working, and even was in the docs some time ago. But this was deleted because we don't want to populate this approach. Problem with this snippet that t and useLingui are not directly connected to each other, and not clean for developers that they are related.

Also if you use SSR rendering i highly discourage you from using any global instances of i18n, here i explained why.

useLingui() simply "subscribe" current component for updates when locale is changed. Without it component using t will not re-render on language switch.

Anyway, for migration purpose using this snippet should be fine.

Other option to consider, if you are working on a web application, consider doing full reload of the app on the language switch. It's small cost of user inconvenience but huge simplification of the code and development process. This way you will be able to use t without any additional hooks.

@wentokay
Copy link

Thanks for the detailed explanation.

We actually opted for a full reload after switching locales like you suggested and it's been working great for our requirements!

@wentokay
Copy link

wentokay commented Feb 21, 2024

Another approach that we're now trying. I haven't tested this specific block of code, but it's the essence of what we're doing. It seems to work for our project because everything is client-side.

Adapted from the react example https://lingui.dev/tutorials/react#setup

import React from "react";
import { createRoot } from "react-dom/client";

import { i18n } from "@lingui/core";
import { I18nProvider } from "@lingui/react";
import { messages } from "./locales/en/messages";
import Inbox from "./Inbox";

i18n.load("en", messages);
i18n.activate("en");

const App = () => (
  <I18nProvider i18n={i18n}>
    <InnerApp />
  </I18nProvider>
);

const InnerApp = () => {
  const { i18n } = useLingui();
  return (
    // re-render the app whenever the locale changes
    <React.Fragment key={i18n.locale}>
      <Inbox />
    </React.Fragment>
  );
}

const root = createRoot(document.getElementById('root'));
root.render(<App />);

@timofei-iatsenko
Copy link
Collaborator

Yes, that would also work. That was actually a default behavior in lingui < 4

@dan-dr
Copy link
Contributor Author

dan-dr commented Feb 21, 2024

Wait but what happens if I do this:

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

() => {
  const { i18n } = useLingui();
  const text = t`Text`;
  const other = i18n._(....);
}

from looking at the macro code, i expect this:

import { i18n } from '@lingui/core';
import { useLingui } from '@lingui/react'; 

() => {
  // name clashes with import from core?
  const { i18n } = useLingui();
  const text = i18n._(...);
  const other = i18n._(....);
}

technically the i18n instances from the import and the useLingui should clash, but from looking at the output js code, it looks like babel doesn't do the import and just uses the instance from useLingui?

Does that means you can technically just do this?

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

() => {
  const { i18n } = useLingui();
  t`Text`;
}

EDIT: https://www.trickster.dev/post/javascript-ast-manipulation-with-babel-untangling-scope-confusion/ says babel uses innermost scope reference when such clashes appear.

Though you still have problem with memoization probably...

@dan-dr
Copy link
Contributor Author

dan-dr commented Feb 21, 2024

I'm thinking you can probably do this

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

() => {
  const { _ } = useLingui();
  
  let a = _`Text`;
  
  let b = React.useMemo(() => { return _`B` }, [_]);
}

// transforms into
import { useLingui } from '@lingui/react';

() => {
  const { _ } = useLingui();
  
  let a = _(...descriptor...);
  
  let b = React.useMemo(() => { return _(...descriptor...) }, [_]);
}

by using scope.getBinding('_') in the macro and then traversing it's referencePaths, if there is a taggedtemplate call, replace it with callexpression and descriptor, and if it's an arrayexpression leave it. other uses should throw the reportUnsupportedSyntax. I'm trying to write it but can't seem to be able to run the project locally :/

@timofei-iatsenko
Copy link
Collaborator

Thanks for digging into it, firstly we are a bit restricted with what babel-macro allow us to do. Secondly we also have SWC plugin which replicate functionality and we should keep them in sync and SWC's plugins API doesn't replicate babel's API especially in part of scopes and references.

I don't say this is not possible, I just say this would open a Pandora's box of weird cases. Also, you need to consider that lingui supports recursive macro expanding, such as:

t`Hello ${plural(count, {single, plural})}`

Any way, if you are willing to try, I can assist you in discord.

@dan-dr
Copy link
Contributor Author

dan-dr commented Feb 22, 2024

Yeah I had a decent attempt that worked the way I mentioned but nesting was an issue. check it here: https://github.com/dan-dr/js-lingui/tree/feat/improved-react-hook-syntax

I'll give it up for now, and might go with the wrapping fragment since we are only client side. feel free to close or keep open for more ideas.

@timofei-iatsenko
Copy link
Collaborator

Well, i picked it up when you finished and was able to make it work.

timofei-iatsenko added a commit to timofei-iatsenko/js-lingui that referenced this issue Feb 22, 2024
timofei-iatsenko added a commit to timofei-iatsenko/js-lingui that referenced this issue Feb 22, 2024
@timofei-iatsenko
Copy link
Collaborator

@dan-dr could you try on your project? Here is how to run it on your local project

#1859

@andrii-bodnar andrii-bodnar linked a pull request Feb 28, 2024 that will close this issue
8 tasks
@timofei-iatsenko
Copy link
Collaborator

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

Successfully merging a pull request may close this issue.

3 participants