lingui / js-lingui

🌍 📖 A readable, automated, and optimized (3 kb) internationalization for JavaScript
https://lingui.dev
MIT License
4.51k stars 378 forks source link

`useLingui` should be "React friendly" #606

Closed theKashey closed 3 years ago

theKashey commented 4 years ago

Is your feature request related to a problem? Please describe. I want to have elements I could use anywhere in my code, so I do thing = <Trans>Hello</Trans>, however sometimes I have to pass this as a string to placeholders and other things. I could use Trans.render to "renderProp" the message, but I would prefer useLingui._ to translate what I have into the string. Unfortunately that is returning [Object object]

Right now API exposed thought i18n is friendly only to core, not react. Well, it is from core.

Describe the solution you'd like i18n._ from useLingui or I18n should understand that some values might be a React.Elements.

Describe alternatives you've considered Well, all I need is stored in the Trans`s props.

if (id.type === Trans) {
   return i18n._(id.props);
}

Additional context Technically, implementation of Trans should be exposed to the end user, so one could call it without using Trans itself.

tricoder42 commented 4 years ago

Hey @theKashey, this proposal makes sense. I guess we need to call renderToString, because i18n._ returns always a string.

Do you have any idea how to solve it so the @lingui/core doesn't need React as a dependency. Some kind of wrapped imported from @lingui/react?

Feel free to sketch your idea and send a PR. Please use next branch as a base.

theKashey commented 4 years ago

Well, renderToString would be an overpower, and well, not an option - you need renderToString only to render "random react components" inside Trans, and they could be random enough to require something(anything!) from context, which would be cut. Ie renderToString is all about isolation.

I do have a function called dereference which could translate Trans to a string, and if it will found another Trans inside it - recursively repeat the operation. Works for 99% cases when you need a "string"

tricoder42 commented 4 years ago

I'm getting a little bit lost. Could you please show me few examples?

For example, why can't you run thing = t'Hello' instead of thing = <Trans>Hello</Trans>? And how the dereference function works, when you render component to string using current component tree?

theKashey commented 4 years ago

For example - let's imagine I have a variable created as you've proposed

const ProductName = t`GitHub`

It is actually not usable until(it's an object) passed through lignui, so I am not able to do this - <h1>{ProductName}</h1>

So I am changing my code to const ProductName = <Trans>GitHub</Trans>. Works this time with React code <h1>{ProductName}</h1> 👍, but if I need it to pass as a string anywhere - 😭.

So in my case I need a string, which could be used anywhere, and there are two options here.

  1. use t everywhere, magically unfold them if used in react. (one could hack React.createElement to apply Trans automatically)
  2. use Trans everywhere, then have an ability to convert Trans to string without using React. The simples way is just to let i18n._ know about Trans
    // literally function I am using for a few months...
    function useLingui (id, values){
      if (id && typeof id === "object") {
        if (id.type === Trans) {
          // in case of trans - just forward props
          return i18n._(id.props);
          // 👉Trans has a postprocessing code which should be added here. Another ticket
        }
        // in other case merge data together
        return i18n._({
          ...id,
          values: {
            ...values,
            ...id.values,
          },
        });
      }
      // or just call as is
      return i18n._(id, values);
    },
tricoder42 commented 4 years ago

In v3 all js macros return translation, not an object:

t`GitHub` === 'GitHub'
// t`GitHub` is the same as i18n._({ id: `GitHub` })

I guess that solves your problem?

So I am changing my code to const ProductName = GitHub.

Why don't you use i18n._ here?

const ProductName = i18._(t`GitHub`) // This is the same as t`GitHub` in v3
theKashey commented 4 years ago

Why don't you use i18n._ here?

Well, there is a cool feature of Lingui - you can just add it into your code, wrapping stuff with Trans.

 const subject = "friend";
 <div>Hello my {subject}</div>

⬇️⬇️⬇️⬇️

 <Trans>Hello my {subject} </Trans>

⬇️⬇️⬇️⬇️

 const subject = <Trans>friend</Trans>;
 <Trans>Hello my {subject} </Trans>

So all "constants", by induction, are expected to be "just wrapped" with Trans, and just work in the corresponding Components without any extra changes. And it actually would, except a few rare cases which expect string as an input.

But you cannot pass Trans component into i18n._, especially with nested Trans component as per example.

tricoder42 commented 4 years ago

So what do you say about upcoming t macro?

 const subject = "friend";
 <div>Hello my {subject}</div>

⬇️⬇️⬇️⬇️

 t`Hello my ${subject}`

⬇️⬇️⬇️⬇️

 const subject = t`friend`;
 t`Hello my ${subject}`
theKashey commented 4 years ago

So - could subject be defined outside the React component, where no context is present and selected language, as well as possible translations are not known?

tricoder42 commented 4 years ago

No, you would have to use defineMessage for "lazy" translations:

 const subject = defineMessage({ message: "friend" })

 t`Hello my ${i18n._(subject)}`

I guess we could improve it here: If you pass message descriptor as a value, library will translate it automatically. If you pass an object, which isn't a descriptor, the library will throw an error.

// Proposal:
 const subject = defineMessage({ message: "friend" })

 t`Hello my ${subject}`
theKashey commented 4 years ago

Yeah. This is a macro moment I was not able to understand - like macro works on imports, and I am getting i18n as a variable from context/renderProps.

Proposal - inject i18n or better replace t by a "React" version via babel:

import {t} from '@lingui/react;

const Component = () => {
  return  t`Hello my ${subject}`;
}
⬇️⬇️⬇️
may be some step, asking you import `useLingui` somowhere from somewhere
⬇️⬇️⬇️
// for any FunctionLikeComponent withing this file....
const Component = () => {
  const {t} = useLingui();
  return  t`Hello my ${subject}`;
}
tricoder42 commented 4 years ago

t macro actually injects import of i18n instance:

https://github.com/lingui/js-lingui/blob/314a5ec6bdb13ee089e7f66b58e9a3408c4b9fa6/packages/macro/test/js-t.ts#L3-L13

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

khmm12 commented 3 years ago

@tricoder42

(may be off topic)

Global i18n instance leads to many issues:

New lingui v3 is definitely big step forward for the most of simple cases and step back at the same time when you need flexibility.

But seems it can be easily solved for everyone by new macros which will provide behaviour similar to v2.

tricoder42 commented 3 years ago

It's not possible to use lingui on server-side anymore. Before it was easy to create isolated instance for request, then pass to any place which requires i18n. Proxy + Continuation Local Storage is quite hacky. i18n. + defineMessage looks awful: i18n.(defineMessage({ message: 'friend ' })) vs i18n._(tfriend).

Not sure what we could do here. React components can read i18n from context, there's no such mechanism for pure js objects. Is there a global object which is specific to current request? Then you could create a simple wrapper, like:

function i18n () {
   const request = getCurrentRequest() // does it exist?
   if (request.i18n == null) request.i18n = setupI18n()
   return request.i18n
}

Then you could configure macros (using runtimeConfigModule) to use your own i18n which would return unique i18n instance per request.

It requires full react application tree reload (here) to refresh messages were translated before with t macro.

It does and I don't think it's a big deal. This isn't a process that user does often and repeatedly. When user visits the page, locale-detector kicks in and selects the appropriate locale. When it fails, user switches the locale and it'll be picked by next time by locale-detector. So, it only happens when 1) locale-detector isn't successful 2) user sets the locale for the first time

khmm12 commented 3 years ago

Not sure what we could do here. React components can read i18n from context, there's no such mechanism for pure js objects. Is there a global object which is specific to current request? Then you could create a simple wrapper, like:

There is no problem to provide instance. The problem is that all macros except defineMessage use imported global instance.

Assume we have async safe getI18n which returns right i18n instance. It can be done with Continuation Local Storage.

v2 ```js function getMessage() { const i18n = getI18n() const title = i18n._(t`Greetings`) } ``` ---transformed---> ```js function getMessage() { const i18n = getI18n() const title = i18n._({ id: 'Greetings' }) } ```
v3 ```js function getMessage() { const title = t`Greetings` } ``` ---transformed---> ```js import { i18n } from '@lingui/core' function getMessage() { const title = i18n._({ id: 'Greetings' }) } ``` getI18n? ```js function getMessage() { const i18n = getI18n() /* Looks like unused variable? */ const title = t`Greetings` } ``` ---> ```js import { i18n } from '@lingui/core' function getMessage() { const i18n = getI18n() const title = i18n._({ id: 'Greetings' }) } ```

Shallow variable declaration may work, but doesn't play well with static analysis tools, because before transformation the variable isn't used anywhere.

React components can read i18n from context

With new macros you meet the same issue as above. You consume i18n from context, but don't use in explicit way.

v3 ```js function Signin() { const { i18n } = useLingui() /* Looks like unused variable */ return (
) } ```
v2 like ```js function Signin() { const { i18n } = useLingui() /* The variable is used bellow */ return (
) } ```

It does and I don't think it's a big deal. This isn't a process that user does often and repeatedly. When user visits the page, locale-detector kicks in and selects the appropriate locale. When it fails, user switches the locale and it'll be picked by next time by locale-detector. So, it only happens when 1) locale-detector isn't successful 2) user sets the locale for the first time

it leaves no choice. For some existing applications migration to v3 becomes impossible, because you either loose ability to change language without loosing application state or have issues with stuck messages.

tricoder42 commented 3 years ago

@khmm12 I guess we could rewrite macros to wrap message descriptors into a function, rather than object method function:

function getMessage() {
  const title = t`Greetings`
}

becomes:

import { translate } from '@lingui/core'

function getMessage() {
  const title = translate({ id: 'Greetings' })
}

where translate (by default) is just a wrapper around i18n._:

import { i18n } from '@lingui/core'
export function translate(...args) {
   return i18n._(...args)
}

but you could also use your own version using "runtimeConfigModule": ["myI8n", "myI18nModule"]. The original code would be the same:

function getMessage() {
  const title = t`Greetings`
}

but this time transformed into:

import { myTranslate } from 'myI18nModule'

function getMessage() {
  const title = myTranslate({ id: 'Greetings' })
}

Now you can write your own version of transalte, which could read the i18n instance from request context:

function myTranslate (...args) {
   const request = getCurrentRequest()
   if (request.i18n == null) request.i18n = setupI18n()
   return request.i18n._(...args)
}

because you either loose ability to change language without loosing application state or have issues with stuck messages.

react-intl actually suggests to use key={locale} in the provider, which does exactly the same — re-renders the whole app. I wonder if there's any state at all, when you switch the locale. I would expect that it happens at the very beginning of app lifecycle. However, I don't have any data to backup this. Would be interesting to see how often and when users (existing vs new users) switch locale

khmm12 commented 3 years ago

@tricoder42

I guess we could rewrite macros to wrap message descriptors into a function, rather than object method function:

It still has downsides:

react-intl actually suggests to use key={locale} in the provider, which does exactly the same — re-renders the whole app

Because of bad architecture 🙂. It reminds me NavLink from react-router which didn't respond to history change inside pure components due to lack of subscriptions. But react-router realized the problem unlike of react-intl, which makes many people crazy. lingui is a great library, there is no reason to follow anti-patterns.

Good thing the problem is macroses only. I'd revise the API again and go with explicit way keeping the new ones (introduced in v3) for backward compatibility and people who want "magic sugar".

tricoder42 commented 3 years ago

No possibility to pass i18n instance explicitly via arguments/class context. No subscriptions and reactivity inside React components, which leads to the need of full React tree reload.

Could you please elaborate? Ideally post a code example of what you'd like to do, but can't neither with current or proposed api.

Because of bad architecture 🙂. It reminds me NavLink from react-router which didn't respond to history change inside pure components due to lack of subscriptions. But react-router realized the problem unlike of react-intl, which makes many people crazy. lingui is a great library, there is no reason to follow anti-patterns.

I'm not following anti-patterns. I just to a conclusion that change of locale is a rare event in lifecycle of an app and by reloading the whole app we can simplify API and use optimization techniques which otherwise wouldn't be possible (e.g. per-locale bundles). History change, on the other hand, happens very often and therefore it wouldn't make a sense to reload the app everytime it changes.

I'd revise the API again and go with explicit way keeping the new ones (introduced in v3) for backward compatibility and people who want "magic sugar".

Feel free to send a draft PR with your suggestions 👍

khmm12 commented 3 years ago

Could you please elaborate? Ideally post a code example of what you'd like to do, but can't neither with current or proposed api.

The funny thing it looks the similar as v2 🙂

Server-side ```js import { t } from '@lingui/macro' export default function template(i18n) { const title = i18n._(t`There should be title`) return html`

{title}

` } ``` ```js import template from './template' export default function handler(req, res) { const { i18n } = req res.send(template(i18n)) } ```
React ```js export default function useLingui() { const context = useContext(LinguiContext) const i18n = useSubscription(useMemo( () => ({ getCurrentValue: () => makeInstance(context.value), subscribe: callback => { i18n.on('change', callback) return () => i18n.off('change', callback) } }), [context] )) return { i18n } } function makeInstance(i18n) { const newInstance = ... // Create new instance to invalidate memoizations return newInstance } ``` ```js import { t } from '@lingui/macro' export default function Form(i18n) { const { i18n } = useLingui() const somethingExpensive = useMemo(() => calculateExpensive(i18n), [i18n]) return (
) } ```
I also know people who generate PDF documents on server-side via `@react-pdf/renderer` and use lingui for localization. > I'm not following anti-patterns. I just to a conclusion that change of locale is a rare event in lifecycle of an app You definitely not, but @lingui/react v3 yes since it's an only way 🙂. > and by reloading the whole app we can simplify API and use optimization techniques which otherwise wouldn't be possible (e.g. per-locale bundles) Force reload can be optional and used where that's an only way. And even more the future of React is Suspense. Using `useTransition` it's possible to change current locale and wait for react tree update with new translations from asynchronous chunk (or chunks?). ([useTransition example](https://codesandbox.io/s/stupefied-architecture-1ekd4?file=/src/index.js)) > Feel free to send a draft PR with your suggestions 👍 I'm not sure how much it may be useful taking into account of simplicity of the suggestion – create a new package with v2 like macroses or move the new macroses to a new package.
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.