alan-eu/react-native-fast-shadow

Crash when rendering Text within ShadowView within Text

Titozzz opened this issue · 4 comments

Titozzz commented

Describe the bug

When rendering something like

<Text>
  <ShadowView>
    <Text>"Foo"</Text> // This will be RCTVirtualizedText and crash
  </ShadowView>
</Text>

This is because in react-native, Views import TextAncestor context and set it to false (https://github.com/facebook/react-native/blob/0fb71630c7d4eff5758f7148ae2d9b1f09bc9d2c/packages/react-native/Libraries/Components/View/View.js#L130)

To reproduce

See above

Expected behavior

No crash

Your environment

N/Applicable.

TLDR: this crash is well identified and I've started a discussion with Meta engineers to know how we are supposed to proceed as importing from 'react-native/Libraries/Text/TextAncestor' isn't safe, this is more to let other people having this issue that it's being looked at.

WadhahEssam commented

@Titozzz why is importing it directly from react native isn't safe?

Titozzz commented

@WadhahEssam well this isn't a public API and could break at any release. If you are fine with that point, you can totally use it. (We do)

  • 👍1
simontreny commented

Hi @Titozzz,

To be sure I understand correctly, could you tell me more about what you're trying to achieve?

  • If you'd like to apply a shadow directly on the inner text element, you shouldn't use ShadowedView (or even View) but rely instead on the textShadowColor/textShadowRadius/textShadowOffset style props (doc). They work well on both iOS and Android and will perform better on iOS than using a view for this.
  • If instead you'd like to apply a shadow on the inline ShadowedView element (I assume it would at least have a background color in this case), then indeed, we'll have to add a TextAncestor context in ShadowedView for it to work. I'm ok to import it even though it's not a public API as we've been relying on it internally for our design system for a while now and it didn't break for many React Native releases. In the meantime, you can probably fix the crash by just wrapping the ShadowedView with a regular View to get the context without having to import the file yourself. Also, note that inline views in Text comes with some downsides: text within the inline view won't be wrapped, you might need to adjust vertical alignment of text and you might get different results between iOS and Android.
Titozzz commented

Not trying to achieve anything special, usually we avoid nesting view within Text for the reasons listed above, but for some reason there were a few that slipped through. And when I replaced all our Views with ShadowedViews it crashed.

It's not a huge deal since I now only replace Views if there are shadow styles + I removed the nested View as they should not have existed. Also just to be sure, I patched the lib so that it wouldn't crash.

This issue was more a FYI than anything else!