React-native-gesture-handler: Gesture handlers shouldn't rely on PropTypes

Created on 3 Apr 2019  Â·  5Comments  Â·  Source: software-mansion/react-native-gesture-handler

I'm having issue with DrawerLayout — its edgeWidth and drawerLockMode props don't work correctly. In fact, they do work just fine in dev mode, but when JS is bundled with dev=false, the whole screen observes swipes to open the drawer — ignoring both props.

Here's what I managed to gather:

  1. This is somehow an issue with dev=false. Native code compilation mode does not seem to matter
  2. This happens on both iOS 12.2 and on older Android device (although with jsc-android)
  3. PanGestureHandler in render seems to ignore both the enabled prop and hitSlop. The props seem to be correct.

Most helpful comment

I think that JS side is still a very small part of a lib and extending it by this array is not a big deal.

All 5 comments

I found the root cause. And it's this package in my babel config:

transform-react-remove-prop-types',

While this is problem with my config… I do think that gesture handlers relying on PropTypes not just for dev-time validation but for its actual functionality is an anti-pattern. Seeing that PropTypes is being phased out in React Native, perhaps it shouldn't be relied upon at all.

Anyway the root cause is that gesture handlers are defined in terms of PropTypes and then throughout createHandler, this is read:

    setNativeProps(updates) {
      const mergedProps = { ...this.props, ...updates };
      const newConfig = filterConfig(
        transformProps ? transformProps(mergedProps) : mergedProps,
        { ...this.constructor.propTypes, ...customNativeProps },
        config
      );
      this._updateGestureHandler(newConfig);
    }

wouldn't it make sense to just add an array whitelist of native props when defining gesture handlers?

This explains why my touchables stopped working in my release builds. I didn't have time to dive this deep into the issue and since I could just revert to the standard RN components I just did that as a quick fix and waited to implement more features with this lib.

@kmagiera or @osdnk, any input on this? 🙂 I could help trying to move the code over to using a whitelist instead of propTypes but don't want to do it if a PR will be closed 🙃. It's less code to maintain now so it is convenient to use propTypes, I get that. Maybe we could get some kind of workaround in place so one wouldn't have to edit prop names in two places? I agree with @radex that relying on a debugging library in production is less than optimal.

Hello,
I want to get rid of propTypes validation too. I think we’ll merge PR with this change immediately. We can no longer rely on propTypes following current trends in RN.

@osdnk Awesome, thanks for the swift reply. I'll get working on it later today I think. In react-native-vector-icons this was done to remove reliance on propTypes. This is what I was planning on doing unless there is another suggestion?

Having an array of prop names would mean a lot of extra code to maintain, every prop name would have to be added/edited/removed in two places as long as there's also flow definitions/PropTypes involved.

I think that JS side is still a very small part of a lib and extending it by this array is not a big deal.

Was this page helpful?
0 / 5 - 0 ratings