Repository: lingui/js-lingui (ts)

Issue:
`render` in `Trans` should be a renderProp
**Describe the bug**
Let's pick example from the docs
```js
<Trans render={({ translation }) => <Icon label={translation} />}>
```

What it is "expected" to be? It's _ducks_ like a render prop, so it would __just__ render an `Icon` with `translation`

What it is? `React.createElement(render, { translation: value })`, so it would be a new, unique `Element` __every render__.

The bug is: that's just not usable at all. It shall be render prop, and could be it, without any breaking compatibity.

**To Reproduce**

<Trans render={({ translation }) => <Icon label={translation} />}>

**Expected behavior**

Should not remount internal component every render


Both v2 and v3 are affected.


** Proposed fix **

https://github.com/lingui/js-lingui/blob/next/packages/react/src/Trans.tsx#L77
```js
  // Component: (props) => <a title={props.translation}>x</a>
  return React.createElement(render as React.ElementType<TransRenderProps>, {
    id,
    translation,
    message
  })
```
👇👇👇👇
```js
  // Component: (props) => <a title={props.translation}>x</a>
  return render({
    id,
    translation,
    message
  })
```

Pull request:
fix: render from prop 
Fixes #607 

Patch:
diff --git a/docs/ref/react.rst b/docs/ref/react.rst
index 6f9b9f06..91c71087 100644
--- a/docs/ref/react.rst
+++ b/docs/ref/react.rst
@@ -18,48 +18,56 @@ Rendering of Translations
 -------------------------
 
 All i18n components render translation as a text without a wrapping tag. This can be
-customized in two different ways: globally: using ``defaultRender`` prop on
-:component:`I18nProvider` component; or locally: using ``render`` prop on i18n
-components.
+customized in three different ways:
+   - globally: using ``defaultComponent`` prop on :component:`I18nProvider` component;
+   - locally: using ``render`` prop or ``component`` on i18 components
 
 Global Configuration
 ^^^^^^^^^^^^^^^^^^^^
 
-Default rendering component can be set using ``defaultRender`` prop in
+Default rendering component can be set using ``defaultComponent`` prop in
 :component:`I18nProvider`. The main use case for this is rendering translations
 in ``<Text>`` component in React Native.
 
-It's possible to pass in either a string for built-in elements (`span`, `h1`),
-React elements or React classes. This prop has the same type as ``render`` prop on
+.. |ss| raw:: html
+
+    <strike>
+
+.. |se| raw:: html
+
+    </strike>
+
+|ss| It's possible to pass in either a string for built-in elements (`span`, `h1`) |se|,
+React elements or React classes. This prop has the same type as ``render`` and ``component`` prop on
 i18n components described below.
 
 Local Configuration
 ^^^^^^^^^^^^^^^^^^^
 
-============= ==================================== ============================
-Prop name     Type                                 Description
-============= ==================================== ============================
-``className`` string                               Class name to be added to ``<span>`` element
-``render``    Element, Component, string, ``null`` Custom wrapper element to render translation
-============= ==================================== ============================
+============= ========================================  ============================
+Prop name     Type                                         Description
+============= ========================================  ============================
+``className`` string                                       Class name to be added to ``<span>`` element
+``render``    `Function(props) -> Element | Component`     Custom wrapper rendered as function
+``component``  Component, ``null``                         Custom wrapper element to render translation
+============= ========================================  ============================
 
 ``className`` is used only for built-in components (when `render` is string).
 
-When ``render`` is **React.Element** or **string** (built-in tags), it is
-cloned with the ``translation`` passed in as its child:
+``Function(props)`` props returns the translation, an id, and a message.
+
+When ``component`` is **React.Element** |ss| or **string** (built-in tags) |se|, it is
+rendered with the ``translation`` passed in as its child:
 
 .. code-block:: jsx
 
-   // built-in tags
-   <Trans render="h1">Heading</Trans>;
-   // renders as <h1>Heading</h1>
+   import { Text } from "react-native";
 
-   // custom elements
-   <Trans render={<Link to="/docs" />}>Link to docs</Trans>;
-   // renders as <Link to="/docs">Link to docs</Link>
+   <Trans component={Text}>Link to docs</Trans>;
+   // renders as <Text>Link to docs</Text>
 
-Using **React.Component** (or stateless component) in ``render`` prop is useful
-to get more control over the rendering of translation. Component passed to
+To get more control over the rendering of translation, use instead the ``render`` method with 
+**React.Component** (or stateless component). Component passed to
 ``render`` will receive the translation value as a ``translation`` prop:
 
 .. code-block:: jsx
@@ -70,15 +78,18 @@ to get more control over the rendering of translation. Component passed to
    </Trans>;
    // renders as <Icon label="Sign in" />
 
-``render`` also accepts ``null`` value to render
+``render`` or ``component`` also accepts ``null`` value to render
 string without wrapping component. This can be used to override
-custom ``defaultRender`` config.
+custom ``defaultComponent`` config.
 
 .. code-block:: jsx
 
    <Trans render={null}>Heading</Trans>;
    // renders as "Heading"
 
+   <Trans component={null}>Heading</Trans>;
+   // renders as "Heading"
+
 Components
 ==========
 
@@ -293,11 +304,11 @@ I18nProvider
    :prop string language: Active language
    :prop string|string[] locales: List of locales used for date/number formatting. Defaults to active language.
    :prop object catalogs: Message catalogs
-   :prop React.Element|React.Class|string defaultRender: Default element to render translation
-   :prop string|Function missing: Custom message to be returned when translation is missing
+   | :prop React.Element | React.Class defaultComponent: Default element to render translation         |
+   | :prop string        | Function missing: Custom message to be returned when translation is missing |
    :prop boolean forceRenderOnLocaleChange: Force re-render when locale changes (default: true)
 
-``defaultRender`` has the same meaning as ``render`` in other i18n
+``defaultComponent`` has the same meaning as ``component`` in other i18n
 components. :ref:`Rendering of translations <rendering-translations>` is explained
 at the beginning of this document.
 
diff --git a/docs/releases/migration-3.rst b/docs/releases/migration-3.rst
index b0087bc8..af527a1d 100644
--- a/docs/releases/migration-3.rst
+++ b/docs/releases/migration-3.rst
@@ -16,9 +16,18 @@ Minimal required versions are:
 
 - ``<I18n>`` render-prop component was removed in favor of :js:func:`useLingui` hook.
 
+- In :component:`I18nProvider`, ``defaultRender`` prop was renamed to
+  ``defaultComponent``, and now only accepts Custom Components
+
 - In :component:`Trans`, ``defaults`` prop was renamed to ``message`` and
   ``description`` to ``comment``.
 
+- In :component:`Trans`, ``render`` prop only accepts render-prop function
+  which is used to render translation.
+
+- In :component:`Trans`, new prop ``component`` accepts React component which
+  is used to render transaltion.
+
 - In :component:`Trans`, ``components`` is now an object, not an array. When
   using the low level API, it allows to name the component placeholders:
 
diff --git a/docs/tutorials/react-native.rst b/docs/tutorials/react-native.rst
index 971fe69d..fde011eb 100644
--- a/docs/tutorials/react-native.rst
+++ b/docs/tutorials/react-native.rst
@@ -207,7 +207,7 @@ As described in the :ref:`reference <rendering-translations>`, by default, trans
   <Text><Trans>Message Inbox</Trans></Text>
 
 
-You'll surely agree the :component:`Text` component looks a little redundant. That's why the :component:`I18nProvider` component accepts a ``defaultRender`` prop. Just supply the :component:`Text` component as the ``defaultRender`` prop and the previous example can be simplified to: 
+You'll surely agree the :component:`Text` component looks a little redundant. That's why the :component:`I18nProvider` component accepts a ``defaultComponent`` prop. Just supply the :component:`Text` component as the ``defaultComponent`` prop and the previous example can be simplified to: 
 
 .. code-block:: jsx
 
diff --git a/package.json b/package.json
index 3155ffac..f29e5b53 100644
--- a/package.json
+++ b/package.json
@@ -1,6 +1,6 @@
 {
   "private": true,
-  "version": "3.0.0-18",
+  "version": "3.0.0-19",
   "author": {
     "name": "Tomáš Ehrlich",
     "email": "tomas.ehrlich@gmail.com"
diff --git a/packages/macro/index.d.ts b/packages/macro/index.d.ts
index e85c5d3d..8bcf5e66 100644
--- a/packages/macro/index.d.ts
+++ b/packages/macro/index.d.ts
@@ -1,6 +1,6 @@
 import type { ComponentType, ReactNode } from "react"
 import type { MessageDescriptor } from "@lingui/core"
-import type { TransRenderType } from "@lingui/react"
+import type { TransRenderType, TransRenderProps } from "@lingui/react"
 
 export function t(
   literals: TemplateStringsArray,
@@ -29,7 +29,8 @@ export function defineMessage(descriptor: MessageDescriptor): MessageDescriptor
 export type TransProps = {
   id?: string
   comment?: string
-  render?: TransRenderType
+  component?: TransRenderType
+  render?: (opts: TransRenderProps) => TransRenderType
 }
 
 export type ChoiceProps = {
diff --git a/packages/react/src/I18nProvider.tsx b/packages/react/src/I18nProvider.tsx
index cc4fdb88..374384ba 100644
--- a/packages/react/src/I18nProvider.tsx
+++ b/packages/react/src/I18nProvider.tsx
@@ -4,7 +4,7 @@ import { TransRenderType } from "./Trans"
 
 type I18nContext = {
   i18n: I18n
-  defaultRender?: TransRenderType
+  defaultComponent?: TransRenderType
 }
 
 export type I18nProviderProps = I18nContext & {
@@ -46,7 +46,7 @@ export function withI18n(o?: object): <P extends { i18n: I18n }>(Component: Comp
 
 export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
   i18n,
-  defaultRender,
+  defaultComponent,
   forceRenderOnLocaleChange = true,
   children,
 }) => {
@@ -56,14 +56,14 @@ export const I18nProvider: FunctionComponent<I18nProviderProps> = ({
    * reference identity and suggested workaround is create a wrapper object every time
    * we need to trigger re-render. See https://reactjs.org/docs/context.html#caveats.
    *
-   * Due to this effect we also pass `defaultRender` in the same context, instead
+   * Due to this effect we also pass `defaultComponent` in the same context, instead
    * of creating a separate Provider/Consumer pair.
    *
    * We can't use useMemo hook either, because we want to recalculate value manually.
    */
   const makeContext = () => ({
     i18n,
-    defaultRender,
+    defaultComponent,
   })
 
   const [context, setContext] = React.useState<I18nContext>(makeContext())
diff --git a/packages/react/src/Trans.tsx b/packages/react/src/Trans.tsx
index 79d1bbad..454dbfac 100644
--- a/packages/react/src/Trans.tsx
+++ b/packages/react/src/Trans.tsx
@@ -4,15 +4,15 @@ import { useLingui } from "./I18nProvider"
 import { formatElements } from "./format"
 
 export type TransRenderProps = {
-  id: string
-  translation: React.ReactNode
-  message: string | null
+  id?: string
+  translation?: React.ReactNode
+  children?: string | any[] | React.ReactNode
+  message?: string | null
 }
 
 export type TransRenderType =
-  | string
+  | React.ComponentType<TransRenderProps>
   | React.ElementType<TransRenderProps>
-  | React.ReactElement
 
 export type TransProps = {
   id: string
@@ -20,12 +20,13 @@ export type TransProps = {
   values: Object
   components: { [key: string]: React.ElementType | any }
   formats?: Object
-  render?: TransRenderType
+  component?: TransRenderType
+  render?: (opts: TransRenderProps) => TransRenderType
 }
 
 export function Trans(props: TransProps) {
-  const { i18n, defaultRender } = useLingui()
-  const { render = defaultRender, id, message, formats } = props
+  const { i18n, defaultComponent } = useLingui()
+  const { render, component, id, message, formats } = props
 
   const values = { ...props.values }
   const components = { ...props.components }
@@ -55,7 +56,7 @@ export function Trans(props: TransProps) {
     })
   }
 
-  const _translation =
+  const _translation: string =
     i18n && typeof i18n._ === "function"
       ? i18n._(id, values, { message, formats })
       : id // i18n provider isn't loaded at all
@@ -64,22 +65,39 @@ export function Trans(props: TransProps) {
     ? formatElements(_translation, components)
     : null
 
-  if (render === null || render === undefined) {
-    return <>{translation}</>
-  } else if (typeof render === "string") {
-    // Built-in element: h1, p
-    return React.createElement(render, {}, translation)
-  } else if (React.isValidElement(render)) {
-    // Element: <p className="lear' />
-    return React.cloneElement(render, {}, translation)
+  const FallbackComponent = defaultComponent || React.Fragment
+
+  // Validation of `render` and `component` props
+  if (render && component) {
+    console.error(
+      "You can't use both `component` and `render` prop at the same time. `component` is ignored."
+    )
+  } else if (render && typeof render !== "function") {
+    console.error(
+      `Invalid value supplied to prop \`render\`. It must be a function, provided ${render}`
+    )
+  } else if (component && typeof component !== "function") {
+    // Apparently, both function components and class components are functions
+    // See https://stackoverflow.com/a/41658173/1535540
+    console.error(
+      `Invalid value supplied to prop \`component\`. It must be a React component, provided ${component}`
+    )
+    return <FallbackComponent>{translation}</FallbackComponent>
+  }
+
+  // Rendering using a render prop
+  if (typeof render === "function") {
+    // Component: render={(props) => <a title={props.translation}>x</a>}
+    return render({
+      id,
+      translation,
+      message,
+    })
   }
 
-  // Component: (props) => <a title={props.translation}>x</a>
-  return React.createElement(render as React.ElementType<TransRenderProps>, {
-    id,
-    translation,
-    message,
-  })
+  // `component` prop has a higher precedence over `defaultComponent`
+  const Component = component || FallbackComponent
+  return <Component>{translation}</Component>
 }
 
 Trans.defaultProps = {


Test patch:
diff --git a/packages/react/src/Trans.test.tsx b/packages/react/src/Trans.test.tsx
index 588cf133..1cfb730f 100644
--- a/packages/react/src/Trans.test.tsx
+++ b/packages/react/src/Trans.test.tsx
@@ -40,6 +40,27 @@ describe("Trans component", function () {
     console.error = originalConsole
   })
 
+  it("should throw a console.error about deprecated usage of string built-in", function () {
+    const originalConsole = console.error
+    console.error = jest.fn()
+
+    renderWithI18n(<Trans render="span" id="Just a text" />)
+    expect(console.error).toHaveBeenCalled()
+
+    renderWithI18n(<Trans render="span" id="Just a text" />)
+    expect(console.error).toHaveBeenCalledTimes(2)
+    console.error = originalConsole
+  })
+
+  it("should throw a console.error if using twice props", function () {
+    const originalConsole = console.error
+    console.error = jest.fn()
+
+    renderWithI18n(<Trans render="div" component="span" id="Just a text" />)
+    expect(console.error).toHaveBeenCalled()
+    console.error = originalConsole
+  })
+
   it("should render default string", function () {
     expect(text(<Trans id="unknown" />)).toEqual("unknown")
 
@@ -85,7 +106,8 @@ describe("Trans component", function () {
   })
 
   it("should render translation inside custom component", function () {
-    const html1 = html(<Trans render={<p className="lead" />} id="Original" />)
+    const Component = (props) => <p className="lead">{props.children}</p>
+    const html1 = html(<Trans component={Component} id="Original" />)
     const html2 = html(
       <Trans
         render={({ translation }) => <p className="lead">{translation}</p>}
@@ -94,7 +116,7 @@ describe("Trans component", function () {
     )
 
     expect(html1).toEqual('<p class="lead">Původní</p>')
-    expect(html2).toEqual(html1)
+    expect(html2).toEqual('<p class="lead">Původní</p>')
   })
 
   it("should render custom format", function () {
@@ -120,14 +142,9 @@ describe("Trans component", function () {
       expect(txt).toEqual("Just a text")
     })
 
-    it("should render with built-in element", function () {
-      const span = html(<Trans render="span" id="Just a text" />)
-      expect(span).toEqual("<span>Just a text</span>")
-    })
-
     it("should render custom element", function () {
-      const element = html(<Trans render={<h1 />} id="Headline" />)
-      expect(element).toEqual("<h1>Headline</h1>")
+      const element = html(<Trans render={({ id, translation }) => <h1 id={id}>{translation}</h1>} id="Headline" />)
+      expect(element).toEqual(`<h1 id="Headline">Headline</h1>`)
     })
 
     it("should render function", function () {
@@ -150,13 +167,39 @@ describe("Trans component", function () {
       })
     })
 
-    it("should take default render element", function () {
+    it("should take defaultComponent prop with a custom component", function () {
+      const ComponentFC: React.FunctionComponent = (props: { children?: React.ReactNode }) => {
+        return (<div>{props.children}</div>)
+      }
       const span = render(
-        <I18nProvider i18n={i18n} defaultRender="p">
+        <I18nProvider i18n={i18n} defaultComponent={ComponentFC}>
           <Trans id="Just a text" />
         </I18nProvider>
       ).container.innerHTML
-      expect(span).toEqual("<p>Just a text</p>")
+      expect(span).toEqual(`<div>Just a text</div>`)
+    })
+  })
+
+  describe("component prop rendering", function() {
+    it("should render class component as simple prop", function () {
+      class ClassComponent extends React.Component {
+        render() {
+          return (
+            <div>Headline</div>
+          )
+        }
+      }
+      const element = html(<Trans component={ClassComponent} id="Headline" />)
+      expect(element).toEqual("<div>Headline</div>")
+    })
+
+    it("should render functional component as simple prop", function () {
+      const ComponentFC: React.FunctionComponent = (props: { id: any, children?: React.ReactNode }) => {
+        const [state] = React.useState("value")
+        return <div id={props.id}>{state}</div>
+      }
+      const element = html(<Trans component={ComponentFC} id="Headline" />)
+      expect(element).toEqual(`<div>value</div>`)
     })
   })
 })