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

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

Open
tpoisseau opened this issue Jul 7, 2023 · 10 comments
Open
Assignees

Comments

@tpoisseau
Copy link
Contributor

tpoisseau commented Jul 7, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

tpoisseau commented Jul 7, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

stepan662 commented Jul 10, 2023

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. #3194

@tpoisseau
Copy link
Contributor Author

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 :

  • param function for tag interpolation
  • any other type for param interpolation

@tpoisseau
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link

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
Copy link

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

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

No branches or pull requests

4 participants