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

Invariant Violation: Touchable child must either be native or forward setNativeProps to a native component #1040

Closed
michalraska opened this issue Apr 27, 2015 · 10 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@michalraska
Copy link

Hi,

when I'm trying to wrap my custom component by TouchableOpacity

        <TouchableOpacity onPress={this._goToThreadDetail}>
          <CommonThreadRow {...item} />
        </TouchableOpacity>

so application falls with error:
Invariant Violation: Touchable child must either be native or forward setNativeProps to a native component

How to solve this?
Thank you.

@michalraska
Copy link
Author

Custom component (in my case CommonThreadRow) has to set native props:

  setNativeProps: function (props: Object) {
    this.refs[COMMON_THREAD_ROW].setNativeProps(props);
  },

  render: function () {
    return (
      <View ref={COMMON_THREAD_ROW}>
        ...
      </View>
    );
  },

But it works only with TouchableHighlight. With TouchableOpacity it does not throw error but method _goToThreadDetail is not called after touch on wrapped view (CommonThreadRow component).

@nicklockwood
Copy link
Contributor

cc: @vjeux

@vjeux
Copy link
Contributor

vjeux commented Apr 27, 2015

The problem with those two components, is that they reach in the internals of the child via setNativeProps and POPAnimationMixin.startAnimation. In the meantime, you can wrap your children into a <View> and it should work fine.

I'm working on a larger proposal that would address those two problems.

The React way is to pass down updated values on every frame as prop and let the child component update on every frame. The issue is that if you don't implement shouldComponentUpdate in your child component (which is highly likely) then you're going to re-render large parts of your app and the animation or update will be really slow.

So, in order to "fix" this, what we do is to take the underlying node and set the values directly. This works when you are passing native components such as View or Image, but if you are passing composite components it falls apart. This is a really unfortunate situation as the biggest strength of React is that you can build your app with those composite components.

My idea is to pass down the current opacity as the style prop as usual, and let it any composite component in between manipulate it and thread it through an eventual native component. Then, the trick is to record on what native element and what style attribute it was eventually applied to.

If you can store this information as metadata of the style value, then anytime you want to update it, you can just list all the affected elements and set their value directly.

In order to make this work, you need the value to be opaque, meaning that all the composite components (which are implemented using arbitrary js) can only move it around but not be able to inspect the value. Thankfully for style, this isn't an issue in practice as all the objects returned from StyleSheet.create are already opaque.

You'll likely hear more from this crazy idea in the next few weeks :)

@michalraska
Copy link
Author

Thank you for very detailed answer.
Afterwards I moved whole logic to the component where is Touchable* wrapping directly View.
I'm looking for hear more and dive more to react native :)

marxsk added a commit to marxsk/react-native-lightbox that referenced this issue Sep 13, 2016
Previously, there was no way how to simply pass arguments from <Lightbox> to
the renderContent(). The only possibility was to create a new component and
utilize activeProps what works fine for simpler cases. But this component has
to be either native or it have to forward setNativeProps. Such forwarding
may be a little more complex as shown at
facebook/react-native#1040
where change of application logic was the solution. Such solution does not
make sense with <Lightbox>.
@Aadesh05
Copy link

Aadesh05 commented Apr 17, 2017

adding to "michalraska" answer solved my problem basically just changed the COMMON_THREAD_ROW as a string instead of object:

setNativeProps(props: Object) {
this.refs['COMMON_THREAD_ROW'].setNativeProps(props)
}
render() {
return (
< Text ref='COMMON_THREAD_ROW'>{this.props.data}
)
}

@coreform
Copy link

@Aadesh05 string refs are bad-practice, so @michalraska was doing it better: https://facebook.github.io/react/docs/refs-and-the-dom.html

Legacy API: String Refs#
If you worked with React before, you might be familiar with an older API where the ref attribute is a string, like "textInput", and the DOM node is accessed as this.refs.textInput. We advise against it because string refs have some issues, are considered legacy, and are likely to be removed in one of the future releases. If you're currently using this.refs.textInput to access refs, we recommend the callback pattern instead.

@gsklee
Copy link

gsklee commented Jun 22, 2017

@vjeux So, umm, where is this crazy idea...? I'm still getting this same error after 2 years...

@petekp
Copy link

petekp commented Jun 30, 2017

I get this error with the following code:

<TouchableHighlight onPress={() => { appSwitchToLink(link.href) }}>
  <View>
    <PAText style={styles.link}>
      {link.title}
    </PAText>
  </View>
</TouchableHighlight>

PAText is a stateless wrapper component of <Text> and seems to be the cause of the issue, despite it being wrapped in a <View>. Making <PAText> a class-based component with a setNativeProps method doesn't resolve the problem. <TouchableOpacity> works just fine here.

@glendenning
Copy link

This resolved the error that I was getting:

<TouchableHighlight onPress={() => console.log('touched')}>
  <View>
    <Element that was in the touchable highlight before />
  </View>
</TouchableHighlight>

Wrapping the internal element within a view tag was magic

@aloifolia
Copy link

WTF? Why is this necessary? And why is it not documented?

marxsk added a commit to marxsk/react-native-lightbox that referenced this issue May 22, 2018
    Previously, there was no way how to simply pass arguments from <Lightbox> to
    the renderContent(). The only possibility was to create a new component and
    utilize activeProps what works fine for simpler cases. But this component has
    to be either native or it have to forward setNativeProps. Such forwarding
    may be a little more complex as shown at
    facebook/react-native#1040
    where change of application logic was the solution. Such solution does not
    make sense with <Lightbox>.
@facebook facebook locked as resolved and limited conversation to collaborators May 29, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests