Support textComponent="" to render raw strings on web

[STRML]

See #987 and particularly #987 (comment) and subsequent comments. We can avoid allocating an additional React.Fragment if the user specifically chooses the empty string (or perhaps "string" or String?) as the textComponent.

[longlho]

Actually I believe internally React creates Fragment if you just return raw string. After making a change locally & adding a test, this comes up:

it('should support rendering raw string if textComponent is falsy', function () {
    const FormattedDate = mockContext(intl);
    const date = Date.now();

    const rendered = shallowDeep(<FormattedDate value={date} textComponent="" />, 2);
    console.log(rendered.type())
    expect(rendered.text()).toBe(intl.formatDate(date));
  })

logs out Symbol(react.fragment)

[longlho]

If anyone can verify that'd be great.

[marcesengel]

Also raw text returns might get messy when inserting JSX via values? How'd we handle that?

[longlho]

I think react 16 supports returning array of children. raw text would just mean no wrapper component I believe.

[marcesengel]

Yes it does, but that requires a key on each element (I don't know how it behaves when there are plain strings in the array).

[longlho]

Yeah it'll trigger a warning but it might be ok? I don't have a strong opinion on this tbh.

[marcesengel]

I'd prefer not to trigger React warnings inside a React library. However I think we could get along with just using the elements indices (see #1204) in our case, because there won't be a reordered rendering of the same elements for 99.9% of the use cases.

But now I wonder what it behaves like if you mix strings/components in an array... 🤣

[longlho]

I think that's an existing issue already right? (if values contains un-keyed react component). Removing the wrapper component shouldn't behave differently from what it does now.

[marcesengel]

Un-keyed components in the values are not an issue as of right now, because we render using

return React.createElement(textComponent, null, ...nodes)

What I wonder about is how React would handle things like:

return [
  'Hello ',
  <b>World</b>
]

Are keys in this case only required for the React elements (e.g. <b>World</b>)?

[marcesengel]

Here we go: CodeSandbox showing you only need to put indices on non-string elements returned in an array.

@longlho I'd prefer this over React.Fragment. What are you thoughts? I could PR tomorrow.

[longlho]

The only reason we kept textComponent as something by default is bc of RN, but since this is a major version bump, I don't mind either way. @redonkulus ?

[marcesengel]

We should probably keep RN support...

[longlho]

we'd still support RN, just that RN users would have to manually set textComponent to React.Fragment

[marcesengel]

Detecting it and setting the root textComponent based on it might be even smoother. What do you think?

[longlho]

that might just be overkill in terms of complexity. I think keeping it as React.Fragment is ok, the perf difference is negligible

[jdahdah]

Just a note that right now it's not possible to use <FormattedMessage /> in combination with react-markdown because that expects a string as a child. If I understand this issue correctly, this would be a use case for returning raw strings.

Example:

<ReactMarkdown>
  <FormattedMessage id="exampleText" />
</ReactMarkdown>

Error:

Failed prop type: Invalid prop `children` of type `object` supplied to `ReactMarkdown`, expected `string`.

[longlho]

hmm interestingly @types/react doesn't allow returning string from functional component so we might have to work around that