Move hitSlop normalization to JS (#4388)

## Description

This moves the parsing and validation of `hitSlop` into one shared JS
module and reduces the platforms to reading a fixed shape.

JS now sends a normalized [left, top, right, bottom, width, height]
array, where null marks an unspecified edge. That layout matches what
the platforms already stored internally (FloatArray(6) on Android,
RNGHHitSlop on Apple), so the native parsers shrink to mapping null →
NaN, plus the DIP→px conversion on Android.

Applied at all four producers: filterConfig (v1 and v2),
prepareConfigForNativeSide (v3), bindSharedValues (v3's UI-thread path)
and the button's web wrapper.



## Test plan

Tested on Android, iOS and Web

Author: coado

packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt

@@ -182,18 +182,6 @@ open class GestureHandler {
     hitSlop!![HIT_SLOP_BOTTOM_IDX] = bottomPad
     hitSlop!![HIT_SLOP_WIDTH_IDX] = width
     hitSlop!![HIT_SLOP_HEIGHT_IDX] = height
-    require(!(hitSlopSet(width) && hitSlopSet(leftPad) && hitSlopSet(rightPad))) {
-      "Cannot have all of left, right and width defined"
-    }
-    require(!(hitSlopSet(width) && !hitSlopSet(leftPad) && !hitSlopSet(rightPad))) {
-      "When width is set one of left or right pads need to be defined"
-    }
-    require(!(hitSlopSet(height) && hitSlopSet(bottomPad) && hitSlopSet(topPad))) {
-      "Cannot have all of top, bottom and height defined"
-    }
-    require(!(hitSlopSet(height) && !hitSlopSet(bottomPad) && !hitSlopSet(topPad))) {
-      "When height is set one of top or bottom pads need to be defined"
-    }
   }
 
   fun setHitSlop(padding: Float?) {
@@ -968,68 +956,44 @@ open class GestureHandler {
       private const val KEY_MANUAL_ACTIVATION = "manualActivation"
       private const val KEY_MOUSE_BUTTON = "mouseButton"
       private const val KEY_HIT_SLOP = "hitSlop"
-      private const val KEY_HIT_SLOP_LEFT = "left"
-      private const val KEY_HIT_SLOP_TOP = "top"
-      private const val KEY_HIT_SLOP_RIGHT = "right"
-      private const val KEY_HIT_SLOP_BOTTOM = "bottom"
-      private const val KEY_HIT_SLOP_VERTICAL = "vertical"
-      private const val KEY_HIT_SLOP_HORIZONTAL = "horizontal"
-      private const val KEY_HIT_SLOP_WIDTH = "width"
-      private const val KEY_HIT_SLOP_HEIGHT = "height"
       private const val KEY_TEST_ID = "testID"
       private const val KEY_CANCELS_JS_RESPONDER = "cancelsJSResponder"
 
+      /**
+       * `hitSlop` arrives already normalized by the JS side as
+       * `[left, top, right, bottom, width, height]`, where `null` marks an edge that was not
+       * specified. Validation of the `width`/`height` combinations happens in JS as well, so all
+       * that is left here is converting the values from DIP to pixels.
+       */
       private fun handleHitSlopProperty(handler: GestureHandler, config: ReadableMap) {
         if (config.isNull(KEY_HIT_SLOP)) {
           handler.setHitSlop(null)
-        } else if (config.getType(KEY_HIT_SLOP) == ReadableType.Number) {
-          val hitSlop = PixelUtil.toPixelFromDIP(config.getDouble(KEY_HIT_SLOP))
-          handler.setHitSlop(
-            hitSlop,
-            hitSlop,
-            hitSlop,
-            hitSlop,
-            GestureHandler.HIT_SLOP_NONE,
-            GestureHandler.HIT_SLOP_NONE,
-          )
+          return
+        }
+
+        // A uniform hit slop stays a plain number on the wire, which spares the bridge the
+        // wrapper it would allocate for an array.
+        if (config.getType(KEY_HIT_SLOP) == ReadableType.Number) {
+          handler.setHitSlop(PixelUtil.toPixelFromDIP(config.getDouble(KEY_HIT_SLOP)))
+          return
+        }
+
+        val hitSlop = config.getArray(KEY_HIT_SLOP)!!
+
+        fun edge(index: Int) = if (hitSlop.isNull(index)) {
+          GestureHandler.HIT_SLOP_NONE
         } else {
-          val hitSlop = config.getMap(KEY_HIT_SLOP)!!
-          var left = GestureHandler.HIT_SLOP_NONE
-          var top = GestureHandler.HIT_SLOP_NONE
-          var right = GestureHandler.HIT_SLOP_NONE
-          var bottom = GestureHandler.HIT_SLOP_NONE
-          var width = GestureHandler.HIT_SLOP_NONE
-          var height = GestureHandler.HIT_SLOP_NONE
-          if (hitSlop.hasKey(KEY_HIT_SLOP_HORIZONTAL)) {
-            val horizontalPad = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_HORIZONTAL))
-            right = horizontalPad
-            left = right
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_VERTICAL)) {
-            val verticalPad = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_VERTICAL))
-            bottom = verticalPad
-            top = bottom
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_LEFT)) {
-            left = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_LEFT))
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_TOP)) {
-            top = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_TOP))
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_RIGHT)) {
-            right = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_RIGHT))
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_BOTTOM)) {
-            bottom = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_BOTTOM))
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_WIDTH)) {
-            width = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_WIDTH))
-          }
-          if (hitSlop.hasKey(KEY_HIT_SLOP_HEIGHT)) {
-            height = PixelUtil.toPixelFromDIP(hitSlop.getDouble(KEY_HIT_SLOP_HEIGHT))
-          }
-          handler.setHitSlop(left, top, right, bottom, width, height)
+          PixelUtil.toPixelFromDIP(hitSlop.getDouble(index))
         }
+
+        handler.setHitSlop(
+          edge(HIT_SLOP_LEFT_IDX),
+          edge(HIT_SLOP_TOP_IDX),
+          edge(HIT_SLOP_RIGHT_IDX),
+          edge(HIT_SLOP_BOTTOM_IDX),
+          edge(HIT_SLOP_WIDTH_IDX),
+          edge(HIT_SLOP_HEIGHT_IDX),
+        )
       }
     }
   }

packages/react-native-gesture-handler/apple/RNGestureHandler.mm

@@ -33,7 +33,24 @@ - (RNGestureHandler *)gestureHandler
 
 static RNGHHitSlop RNGHHitSlopEmpty = {NAN, NAN, NAN, NAN, NAN, NAN};
 
-#define RNGH_HIT_SLOP_GET(key) (prop[key] == nil ? NAN : [prop[key] doubleValue])
+// `hitSlop` reaches the native side already normalized by JS into
+// `[left, top, right, bottom, width, height]`, where `null` marks an unspecified edge.
+typedef NS_ENUM(NSUInteger, RNGHHitSlopIndex) {
+  RNGHHitSlopIndexLeft = 0,
+  RNGHHitSlopIndexTop,
+  RNGHHitSlopIndexRight,
+  RNGHHitSlopIndexBottom,
+  RNGHHitSlopIndexWidth,
+  RNGHHitSlopIndexHeight,
+  RNGHHitSlopIndexCount,
+};
+
+static CGFloat RNGHHitSlopEdge(NSArray *hitSlop, RNGHHitSlopIndex index)
+{
+  id value = hitSlop[index];
+  return [value isKindOfClass:[NSNumber class]] ? [value doubleValue] : NAN;
+}
+
 #define RNGH_HIT_SLOP_IS_SET(hitSlop) \
   (!isnan(hitSlop.left) || !isnan(hitSlop.right) || !isnan(hitSlop.top) || !isnan(hitSlop.bottom))
 #define RNGH_HIT_SLOP_INSET(key) (isnan(hitSlop.key) ? 0. : hitSlop.key)
@@ -166,32 +183,23 @@ - (void)updateConfig:(NSDictionary *)config
     _cancelsJSResponder = [RCTConvert BOOL:prop];
   }
 
+  // A cleared hit slop arrives as six unset slots rather than as `null`, because the TurboModule
+  // bridge drops null-valued keys on this platform. A missing key still means the property was not
+  // part of this update, and leaves the previous value alone.
   prop = config[@"hitSlop"];
   if ([prop isKindOfClass:[NSNumber class]]) {
-    _hitSlop.left = _hitSlop.right = _hitSlop.top = _hitSlop.bottom = [prop doubleValue];
-  } else if ([prop isKindOfClass:[NSNull class]]) {
+    // A uniform hit slop stays a plain number on the wire, skipping the array wrapper.
     _hitSlop = RNGHHitSlopEmpty;
+    _hitSlop.left = _hitSlop.right = _hitSlop.top = _hitSlop.bottom = [prop doubleValue];
+  } else if ([prop isKindOfClass:[NSArray class]]) {
+    _hitSlop.left = RNGHHitSlopEdge(prop, RNGHHitSlopIndexLeft);
+    _hitSlop.top = RNGHHitSlopEdge(prop, RNGHHitSlopIndexTop);
+    _hitSlop.right = RNGHHitSlopEdge(prop, RNGHHitSlopIndexRight);
+    _hitSlop.bottom = RNGHHitSlopEdge(prop, RNGHHitSlopIndexBottom);
+    _hitSlop.width = RNGHHitSlopEdge(prop, RNGHHitSlopIndexWidth);
+    _hitSlop.height = RNGHHitSlopEdge(prop, RNGHHitSlopIndexHeight);
   } else if (prop != nil) {
-    _hitSlop.left = _hitSlop.right = RNGH_HIT_SLOP_GET(@"horizontal");
-    _hitSlop.top = _hitSlop.bottom = RNGH_HIT_SLOP_GET(@"vertical");
-    _hitSlop.left = RNGH_HIT_SLOP_GET(@"left");
-    _hitSlop.right = RNGH_HIT_SLOP_GET(@"right");
-    _hitSlop.top = RNGH_HIT_SLOP_GET(@"top");
-    _hitSlop.bottom = RNGH_HIT_SLOP_GET(@"bottom");
-    _hitSlop.width = RNGH_HIT_SLOP_GET(@"width");
-    _hitSlop.height = RNGH_HIT_SLOP_GET(@"height");
-    if (isnan(_hitSlop.left) && isnan(_hitSlop.right) && !isnan(_hitSlop.width)) {
-      RCTLogError(@"When width is set one of left or right pads need to be defined");
-    }
-    if (!isnan(_hitSlop.width) && !isnan(_hitSlop.left) && !isnan(_hitSlop.right)) {
-      RCTLogError(@"Cannot have all of left, right and width defined");
-    }
-    if (isnan(_hitSlop.top) && isnan(_hitSlop.bottom) && !isnan(_hitSlop.height)) {
-      RCTLogError(@"When height is set one of top or bottom pads need to be defined");
-    }
-    if (!isnan(_hitSlop.height) && !isnan(_hitSlop.top) && !isnan(_hitSlop.bottom)) {
-      RCTLogError(@"Cannot have all of top, bottom and height defined");
-    }
+    _hitSlop = RNGHHitSlopEmpty;
   }
 }
 

packages/react-native-gesture-handler/apple/RNGestureHandlerButtonComponentView.mm

@@ -290,12 +290,16 @@ - (NSDictionary *)buildManagedHandlerConfig:(const RNGestureHandlerButtonProps &
   // and the handler keeps its unset default instead of hit-testing against an identical frame.
   if (props.gestureHitSlop.top != 0 || props.gestureHitSlop.left != 0 || props.gestureHitSlop.bottom != 0 ||
       props.gestureHitSlop.right != 0) {
-    config[@"hitSlop"] = @{
-      @"top" : @(props.gestureHitSlop.top),
-      @"left" : @(props.gestureHitSlop.left),
-      @"bottom" : @(props.gestureHitSlop.bottom),
-      @"right" : @(props.gestureHitSlop.right),
-    };
+    // Matches the normalized `[left, top, right, bottom, width, height]` layout the JS side sends;
+    // the button only exposes the four edges, so width and height are always unset.
+    config[@"hitSlop"] = @[
+      @(props.gestureHitSlop.left),
+      @(props.gestureHitSlop.top),
+      @(props.gestureHitSlop.right),
+      @(props.gestureHitSlop.bottom),
+      [NSNull null],
+      [NSNull null],
+    ];
   }
 
   return config;

packages/react-native-gesture-handler/src/__tests__/hitSlop.test.ts

@@ -0,0 +1,177 @@
+import type { HitSlop } from '../handlers/gestureHandlerCommon';
+import { normalizeHitSlop } from '../handlers/hitSlop';
+
+describe('normalizeHitSlop', () => {
+  test('keeps `undefined` out of the payload and sends `null` as unset slots', () => {
+    // `undefined` keeps the property out of partial config updates. `null` has to
+    // travel as six unset slots, since the iOS bridge drops null-valued keys.
+    expect(normalizeHitSlop(undefined)).toBeUndefined();
+    expect(normalizeHitSlop(null)).toEqual([
+      null,
+      null,
+      null,
+      null,
+      null,
+      null,
+    ]);
+  });
+
+  test('is idempotent', () => {
+    // Normalizing an already normalized value must not empty it out.
+    const normalized = normalizeHitSlop({ horizontal: -10, top: -5 });
+
+    expect(normalizeHitSlop(normalized)).toEqual([
+      -10,
+      -5,
+      -10,
+      null,
+      null,
+      null,
+    ]);
+    expect(normalizeHitSlop(normalizeHitSlop(-10))).toBe(-10);
+  });
+
+  test('forwards a uniform hit slop as a plain number', () => {
+    // Left as a number so the bridge does not allocate an array wrapper for it;
+    // every platform expands it into four equal edges itself.
+    expect(normalizeHitSlop(-10)).toBe(-10);
+    expect(normalizeHitSlop(0)).toBe(0);
+  });
+
+  test('marks unspecified edges as `null`', () => {
+    expect(normalizeHitSlop({})).toEqual([null, null, null, null, null, null]);
+    expect(normalizeHitSlop({ left: -10 })).toEqual([
+      -10,
+      null,
+      null,
+      null,
+      null,
+      null,
+    ]);
+  });
+
+  test('treats an explicitly `undefined` edge as unspecified', () => {
+    // Without this, the value would reach the platforms as `0` and shrink the view.
+    expect(normalizeHitSlop({ left: undefined, top: -5 })).toEqual([
+      null,
+      -5,
+      null,
+      null,
+      null,
+      null,
+    ]);
+  });
+
+  test('expands `horizontal` and `vertical`', () => {
+    expect(normalizeHitSlop({ horizontal: -10 })).toEqual([
+      -10,
+      null,
+      -10,
+      null,
+      null,
+      null,
+    ]);
+    expect(normalizeHitSlop({ vertical: -10 })).toEqual([
+      null,
+      -10,
+      null,
+      -10,
+      null,
+      null,
+    ]);
+    expect(normalizeHitSlop({ horizontal: -10, vertical: -5 })).toEqual([
+      -10,
+      -5,
+      -10,
+      -5,
+      null,
+      null,
+    ]);
+  });
+
+  test('lets an explicit edge win over the shorthand', () => {
+    expect(normalizeHitSlop({ horizontal: -10, left: -20 })).toEqual([
+      -20,
+      null,
+      -10,
+      null,
+      null,
+      null,
+    ]);
+    expect(normalizeHitSlop({ vertical: -10, bottom: -20 })).toEqual([
+      null,
+      -10,
+      null,
+      -20,
+      null,
+      null,
+    ]);
+  });
+
+  test('carries `width` and `height` through', () => {
+    expect(normalizeHitSlop({ left: 0, width: 20 })).toEqual([
+      0,
+      null,
+      null,
+      null,
+      20,
+      null,
+    ]);
+    expect(normalizeHitSlop({ bottom: 0, height: 20 })).toEqual([
+      null,
+      null,
+      null,
+      0,
+      null,
+      20,
+    ]);
+  });
+
+  test('rejects invalid `width` and `height` combinations', () => {
+    expect(() =>
+      normalizeHitSlop({ left: 0, right: 0, width: 20 } as HitSlop)
+    ).toThrow("cannot have all of 'left', 'right' and 'width' defined");
+
+    expect(() => normalizeHitSlop({ width: 20 } as HitSlop)).toThrow(
+      "when 'width' is defined, either 'left' or 'right' has to be defined"
+    );
+
+    expect(() =>
+      normalizeHitSlop({ top: 0, bottom: 0, height: 20 } as HitSlop)
+    ).toThrow("cannot have all of 'top', 'bottom' and 'height' defined");
+
+    expect(() => normalizeHitSlop({ height: 20 } as HitSlop)).toThrow(
+      "when 'height' is defined, either 'top' or 'bottom' has to be defined"
+    );
+  });
+
+  test('rejects a negative `width` or `height`', () => {
+    expect(() => normalizeHitSlop({ left: 0, width: -20 } as HitSlop)).toThrow(
+      "'width' cannot be negative"
+    );
+
+    expect(() => normalizeHitSlop({ top: 0, height: -20 } as HitSlop)).toThrow(
+      "'height' cannot be negative"
+    );
+  });
+
+  test('allows a zero `width` or `height`', () => {
+    // An empty hit area is degenerate but coherent, and a hit slop animated
+    // from zero upwards passes through it.
+    expect(normalizeHitSlop({ left: 0, width: 0 })).toEqual([
+      0,
+      null,
+      null,
+      null,
+      0,
+      null,
+    ]);
+  });
+
+  test('counts a shorthand as defining both of its edges', () => {
+    // `horizontal` fills in both `left` and `right`, which conflicts with `width`.
+    expect(() =>
+      normalizeHitSlop({ horizontal: -10, width: 20 } as HitSlop)
+    ).toThrow("cannot have all of 'left', 'right' and 'width' defined");
+  });
+});