tolgee / tolgee-js

Tolgee JavaScript libraries monorepo
https://tolgee.io
MIT License
231 stars 28 forks source link

Tolgee react: Please make possible to use jsx as param interpolation #3219

Open tpoisseau opened 1 year ago

tpoisseau commented 1 year ago

https://stackblitz.com/edit/gpbse5j-tolgee-jsx-param?file=src%2FApp.tsx

I made you a complete example of the inability to bind jsx as param to the T component (same thing apply on t method. in fact, I tested and it work with t method)

TLDR

<T
  keyName="..."
  params={{
    jsxDate: (
      <b>
        <FormattedDate value={new Date()} />
      </b>
    )
  }}>
  test with jsx date : {jsxDate}'
</T>;

T render test with jsx date : instead test with jsx date : <b>2023/07/07<b>

Workaround is to rely on Tag interpolation but I would be able to not expose to the translator the underlying decoration or format. they just need to take care something will be inserted here in the translation.

And because of no support self-close tag, it's quite difficult to understand the syntax.

tpoisseau commented 1 year ago

Another workaround I found is to use t method instead, it's working well. Except the typing do not support ReactNode as param value

tpoisseau commented 1 year ago

And about ICU syntax, we must define if we use formatting date/number in ICU or we handle it with react-intl helpers. I prefer handle in source code instead freeze it in translation how to format data. For me, runtime is responsible of how format data, not translator.

So, in translation ICU format should just use for placeholder, and plural. format date and numbers should be defined in source code.

Now we have both, majority source code

stepan662 commented 1 year ago

This probably only works by accident in t function, it was not intended. But as you describe it it make sense. I'll look into it.

stepan662 commented 1 year ago

There is a clash with tag interpolation in T component. All the params of T component which include a React node are transformed to a function, so it is compatible with Format.JS interpolation.

<T params={{
  b: <b />
}}>

// is basically shortcut to:

<T params={{
  b: (children) => <b>{children}</b>
}}>

Even though <b></b> and {b} are essentialy the same thing, with Format.JS tags it's not possible to support both. The t function doesn't support tag interpolation, so it works there.

I already plan to use custom tag parser instead of Format.JS, because of self-closing tags and other issues. And in our custom tag parser this should be solvable.

However there will be a breaking change, so it's currently open and we'll see if we have more breaking stuff to put into the version 6. https://github.com/tolgee/tolgee-js/pull/3194

tpoisseau commented 1 year ago

Thank you for your answer

react-intl (what we used before for translations) is part of Format.JS ecosystem and rely on it.

They don't have shortcut jsx syntax for tag-insterpolation and they just ask transform-callback for tags, like you https://formatjs.io/docs/react-intl/components#rich-text-formatting

So, if you agree for jsx param binding, the best thing to do is get rid of the actual shortcut syntax for tag interpolation. I think it will be much more clear syntax and intention :

tpoisseau commented 1 year ago

Hello, I will complete a bit the issue about mixing t and jsx.

I know it's not a planed usage but it's my workaround. Components where I use t with jsx params, React warns about "Each child in list should have a unique key prop". So now, I put key on each of my jsx param to not have the warning.

t(intlId, {
  date: (
    <b>
      <FormattedDate value={date} />
    </b>
  ),
  place: <b>{place}</b>,
})

It's not explicit as user of t function, jsx will be rendered as a child list (in react meanings). So, no key, react complains in console.

Workaround :

t(intlId, {
  date: (
    <b key="date">
      <FormattedDate value={date} />
    </b>
  ),
  place: <b key="place">{place}</b>,
})
heitorsilva commented 1 year ago

I'm having a Typescript lint problem with @tpoisseau solution:

const privacyPolicyLink = () =>
    (
      <Hyperlink key="privacyLink" to="/privacy">
        <T ns="common">here</T>
      </Hyperlink>
    )

return (
  <>
    <Paragraph>{t("text_p16", "", { ns: "terms", privacyPolicyLink: privacyPolicyLink() })}</Paragraph>
  </>
)

results in Type 'Element' is not assignable to type 'string | number | bigint | boolean | Date | null | undefined'.ts(2322)

I can only silence this with

const privacyPolicyLink = () =>
    (
      <Hyperlink key="privacyLink" to="/privacy">
        <T ns="common">here</T>
      </Hyperlink>
    )

return (
  <>
    {/*
       // @ts-ignore */}
    <Paragraph>{t("text_p16", "", { ns: "terms", privacyPolicyLink: privacyPolicyLink() })}</Paragraph>
  </>
)

any other suggestions?

tpoisseau commented 1 year ago

Here is our type definition file for tolgee types override :

/* eslint-disable @typescript-eslint/consistent-type-definitions,@typescript-eslint/no-unnecessary-type-arguments */
import type { CombinedOptions, TranslateProps } from '@tolgee/core';
import type { ReactNode } from 'react';

import type { TK as MaybeTK } from './_translations_keys';

type TK = 0 extends 1 & MaybeTK ? never : MaybeTK;

type DFT =
  | null
  | undefined
  | string
  | number
  | boolean
  | bigint
  | Date
  | ReactNode;
// eslint-disable-next-line @typescript-eslint/no-unused-vars
type TF<T = DFT, R = string, K = TK> = {
  (key: TK, defaultValue?: string, options?: CombinedOptions<DFT>): R;
  (key: TK, options?: CombinedOptions<DFT>): R;
  (props: TranslateProps<DFT, TK>): R;
};

declare global {
  export type TranslationKey = TK;
}

declare module '@tolgee/core/lib/types' {
  export type TranslationKey = TK;

  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  export type TFnType<T = DFT, R = string, K = TK> = TF<DFT, R, TK>;

  export type TranslateParams<T = DFT> = Record<string, T>;
}

declare module '@tolgee/core' {
  export type TranslationKey = TK;

  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  export type TFnType<T = DFT, R = string, K = TK> = TF<DFT, R, TK>;
}

declare module '@tolgee/web/lib/types' {
  export type TranslationKey = TK;

  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  export type TFnType<T = DFT, R = string, K = TK> = TF<DFT, R, TK>;

  // eslint-disable-next-line @typescript-eslint/no-unused-vars
  export type UseTranslateResult<T = DFT, R = string, K = TK> = {
    t: TFnType<DFT, R, TK>;
    isLoading: boolean;
  };
}

declare module '@tolgee/react' {
  export interface UseTranslateResult {
    t: TF<DFT, string, TK>;
  }
}

It add what we may pass to tolgee translate params and force typesafety on key used (and autocomplete)

kalimantos commented 1 year ago

I think the quickest solution is to make the format-icu lib use directly icu-messageformat-parser instead of intl-messageformat and use almost the same intl-messageformat code except for some case. For example (https://github.com/formatjs/formatjs/blob/main/packages/intl-messageformat/src/formatters.ts#L159) when an "argument" element is found should check if is a function (the one created here https://github.com/tolgee/tolgee-js/blob/main/packages/react/src/tagsTools.tsx#L22)

kalimantos commented 1 year ago

If you guys want to use jsx param you can try @artshell/tolgee-format-icu-jsx-param. Just change the import and it's done.

- import { FormatIcu } from '@tolgee/format-icu';
+ import { FormatIcu } from '@artshell/tolgee-format-icu-jsx-param';

More info here https://github.com/Artshell-company/tolgee-format-icu-jsx-param#readme