Move hitSlop normalization to JS - #4388
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change normalizes hit-slop inputs into a canonical six-element array. Legacy, v3, web, Android, and Apple paths now consume this format. Tests cover normalization, validation, shared values, wiring, and nullish behavior. ChangesHit-slop normalization
Sequence Diagram(s)sequenceDiagram
participant GestureConfig
participant normalizeHitSlop
participant NativeConfig
participant GestureHandler
GestureConfig->>normalizeHitSlop: raw hitSlop value
normalizeHitSlop-->>GestureConfig: canonical six-element array
GestureConfig->>NativeConfig: normalized hitSlop
NativeConfig->>GestureHandler: apply hit-slop tuple
GestureHandler-->>GestureConfig: updated gesture bounds
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR centralizes hitSlop parsing/validation in a shared JS module and changes the native/web consumers to accept a single canonical shape ([left, top, right, bottom, width, height] with null for “unset”), reducing platform-specific parsing logic.
Changes:
- Introduces
normalizeHitSlop+CanonicalHitSlopin JS and updates config producers (v1/v2filterConfig, v3 native config prep, v3 shared-value UI-thread updates, and the web button wrapper) to emit the canonical array. - Simplifies native (Android/iOS)
hitSlopparsing to fixed-index reads, plus DIP→px conversion on Android. - Updates web handler types and hit-testing logic to consume the canonical array and adds targeted Jest tests to lock the wire contract.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react-native-gesture-handler/src/web/interfaces.ts | Switches web config types from object HitSlop to CanonicalHitSlop. |
| packages/react-native-gesture-handler/src/web/handlers/GestureHandler.ts | Updates web handler to store/consume canonical hitSlop arrays; removes local validation. |
| packages/react-native-gesture-handler/src/v3/hooks/utils/reanimatedUtils.ts | Normalizes hitSlop when pushed via UI-thread shared value updates. |
| packages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.ts | Normalizes hitSlop in v3 config preparation (including shared values). |
| packages/react-native-gesture-handler/src/utils.ts | Adds isWorkletRuntime helper for worklet-vs-RN runtime detection. |
| packages/react-native-gesture-handler/src/handlers/utils.ts | Normalizes hitSlop in v1/v2 filterConfig. |
| packages/react-native-gesture-handler/src/handlers/hitSlop.ts | Adds shared canonical hitSlop type + normalization/validation logic (worklet-safe). |
| packages/react-native-gesture-handler/src/components/GestureHandlerButton.web.tsx | Normalizes button hitSlop before passing to the web module. |
| packages/react-native-gesture-handler/src/tests/hitSlopWiring.test.ts | Adds wiring tests for normalization at multiple producers. |
| packages/react-native-gesture-handler/src/tests/hitSlopSharedValue.test.ts | Adds test ensuring UI-thread shared-value path normalizes hitSlop. |
| packages/react-native-gesture-handler/src/tests/hitSlop.test.ts | Adds unit tests for normalizeHitSlop, including worklet-runtime behavior. |
| packages/react-native-gesture-handler/apple/RNGestureHandlerButtonComponentView.mm | Updates button hitSlop config emission to the canonical array shape. |
| packages/react-native-gesture-handler/apple/RNGestureHandler.mm | Updates iOS handler hitSlop parsing to read canonical array indices. |
| packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt | Updates Android hitSlop parsing to read canonical array indices and convert DIP→px. |
Suppressed comments (1)
packages/react-native-gesture-handler/src/tests/hitSlop.test.ts:205
- These assignments also rely on
globalThis.__RUNTIME_KIND/_WORKLETtypings. Once a localgis introduced, use it consistently here to avoid TS errors.
test('reports instead of throwing', () => {
// 2 is the UI runtime.
globalThis.__RUNTIME_KIND = 2;
expectReported();
});
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (Array.isArray(hitSlop)) { | ||
| return hitSlop; | ||
| } |
There was a problem hiding this comment.
The only accepted array by types is the one from CanonicalHitSlop which is exactly what the method returns so there is no need for runtime check.
| const runtimeKind: number | undefined = globalThis.__RUNTIME_KIND; | ||
|
|
||
| if (runtimeKind !== undefined) { | ||
| return runtimeKind !== RUNTIME_KIND_REACT_NATIVE; | ||
| } | ||
|
|
||
| return globalThis._WORKLET === true; | ||
| } |
There was a problem hiding this comment.
There is a dev dependency on react-native-reanimated
| let consoleError: jest.SpyInstance; | ||
| const initialRuntimeKind = globalThis.__RUNTIME_KIND; | ||
|
|
||
| beforeEach(() => { | ||
| consoleError = jest.spyOn(console, 'error').mockImplementation(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| globalThis.__RUNTIME_KIND = initialRuntimeKind; | ||
| globalThis._WORKLET = undefined; | ||
| consoleError.mockRestore(); | ||
| }); |
There was a problem hiding this comment.
This is the same as above. We have dev dependency on reanimated
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.ts (1)
110-117: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse
maybeUnpackValuefor the shared-value unwrapping.Line 102 in the same function already unwraps through
maybeUnpackValue. The inlineReanimated?.isSharedValue(value) ? value.value : valueduplicates that rule. Use the helper so both call sites stay in sync.♻️ Proposed refactor
- const unpackedValue = Reanimated?.isSharedValue(value) - ? value.value - : value; + const unpackedValue = maybeUnpackValue(value); (filteredConfig as Record<string, unknown>)[key] = key === 'hitSlop' ? normalizeHitSlop(unpackedValue as HitSlop) : unpackedValue;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.ts` around lines 110 - 117, In the config filtering logic, replace the inline Reanimated shared-value check and value access before the hitSlop handling with the existing maybeUnpackValue helper already used earlier in the same function. Keep the subsequent normalizeHitSlop and filteredConfig assignment behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt`:
- Around line 988-1003: Update the hitSlop reader around the local edge function
and handler.setHitSlop call to validate each tuple entry before accessing it:
treat out-of-bounds or non-numeric values as GestureHandler.HIT_SLOP_NONE,
matching Apple behavior for malformed arrays. Use ReadableArray bounds/type
checks before getDouble so malformed hitSlop configuration cannot crash native
updates.
In `@packages/react-native-gesture-handler/src/web/handlers/GestureHandler.ts`:
- Around line 879-915: Add web hit-slop coverage in GestureHandler.test.ts for
zero-valued edges, explicitly null edges, and combinations of
left/right/top/bottom with width/height sizing. Ensure the tests validate the
bounds computed by the hit-slop logic in GestureHandler, then run yarn lint:js,
yarn format:js, yarn ts-check, and yarn test GestureHandler.test.ts with
dependencies installed.
---
Nitpick comments:
In `@packages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.ts`:
- Around line 110-117: In the config filtering logic, replace the inline
Reanimated shared-value check and value access before the hitSlop handling with
the existing maybeUnpackValue helper already used earlier in the same function.
Keep the subsequent normalizeHitSlop and filteredConfig assignment behavior
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 68c6aa8c-5054-4c13-bc42-508a04972fe0
📒 Files selected for processing (14)
packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.ktpackages/react-native-gesture-handler/apple/RNGestureHandler.mmpackages/react-native-gesture-handler/apple/RNGestureHandlerButtonComponentView.mmpackages/react-native-gesture-handler/src/__tests__/hitSlop.test.tspackages/react-native-gesture-handler/src/__tests__/hitSlopSharedValue.test.tspackages/react-native-gesture-handler/src/__tests__/hitSlopWiring.test.tspackages/react-native-gesture-handler/src/components/GestureHandlerButton.web.tsxpackages/react-native-gesture-handler/src/handlers/hitSlop.tspackages/react-native-gesture-handler/src/handlers/utils.tspackages/react-native-gesture-handler/src/utils.tspackages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.tspackages/react-native-gesture-handler/src/v3/hooks/utils/reanimatedUtils.tspackages/react-native-gesture-handler/src/web/handlers/GestureHandler.tspackages/react-native-gesture-handler/src/web/interfaces.ts
| 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), | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline packages/react-native-gesture-handler/src/handlers/hitSlop.ts --items all
sed -n '112,146p' packages/react-native-gesture-handler/src/handlers/hitSlop.ts
ast-grep outline packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt --items all
sed -n '976,1004p' packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt
rg -n -C 4 'normalizeHitSlop|hitSlop' \
packages/react-native-gesture-handler/src/__tests__/hitSlopSharedValue.test.ts \
packages/react-native-gesture-handler/src/__tests__/hitSlopWiring.test.tsRepository: software-mansion/react-native-gesture-handler
Length of output: 14806
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect Apple hitSlop handling and shared code around normalizeHitSlop callers.
fd -i 'hit[sS]lop' packages/react-native-gesture-handler/{apple,android,src} | sed -n '1,120p'
rg -n -C 6 'normalizeHitSlop|handleHitSlopProperty|setHitSlop|READABLE_NILLABLE_NUMBER_ARRAY_INDEX|NAN' packages/react-native-gesture-handler/{apple,android,src}
# Check the exact Android/Apple hitSlop parsing files with line numbers.
python3 - <<'PY'
import pathlib
for root in ['apple', 'android', 'src']:
p = pathlib.Path('packages/react-native-gesture-handler') / root
files = []
for f in p.rglob('*'):
if f.is_file() and ('hit' in f.name.lower() or 'slop' in f.name.lower()):
files.append(str(f).replace('packages/react-native-gesture-handler/', '', 1))
for f in sorted(files):
if 'normalize' in open(f.read_text(errors='ignore'), encoding='utf-8').read().lower():
print(f'--- {f} ---')
PYRepository: software-mansion/react-native-gesture-handler
Length of output: 50401
Validate each tuple entry before reading it.
normalizeHitSlop returns raw arrays without a six-element/numeric-item guard. This native reader accesses every tuple index with isNull/getDouble, while Apple treats shorter arrays and non-number entries as missing. Add ReadableArray bounds/type handling or reject malformed hitSlop in JS so config updates cannot crash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@packages/react-native-gesture-handler/android/src/main/java/com/swmansion/gesturehandler/core/GestureHandler.kt`
around lines 988 - 1003, Update the hitSlop reader around the local edge
function and handler.setHitSlop call to validate each tuple entry before
accessing it: treat out-of-bounds or non-numeric values as
GestureHandler.HIT_SLOP_NONE, matching Apple behavior for malformed arrays. Use
ReadableArray bounds/type checks before getDouble so malformed hitSlop
configuration cannot crash native updates.
There was a problem hiding this comment.
The method returns and accepts CanonicalHitSlop array only so I don't think we have to introduce additional runtime checks
| if (hitSlop === undefined || hitSlop === null) { | ||
| return hitSlop; | ||
| } | ||
|
|
||
| if (Array.isArray(hitSlop)) { | ||
| return hitSlop; | ||
| } |
There was a problem hiding this comment.
| if (hitSlop === undefined || hitSlop === null) { | |
| return hitSlop; | |
| } | |
| if (Array.isArray(hitSlop)) { | |
| return hitSlop; | |
| } | |
| if (hitSlop === undefined || hitSlop === null || Array.isArray(hitSlop)) { | |
| return hitSlop; | |
| } |
| if (configKey === 'runOnJS') { | ||
| updateGestureHandlerConfig(handlerTag, { | ||
| dispatchesReanimatedEvents: shouldUseReanimatedDetector && !value, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| updateGestureHandlerConfig(handlerTag, { | ||
| [configKey]: | ||
| configKey === 'hitSlop' ? normalizeHitSlop(value as HitSlop) : value, | ||
| }); |
There was a problem hiding this comment.
Maybe switch(configKey) or another if? That way all options would be handled in the same manner. But that's not strong opinion and I'm also fine with what it is now.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/react-native-gesture-handler/src/v3/types/ConfigTypes.ts`:
- Line 77: Resolve the failing Jest cases in hitSlop.test.ts and
hitSlopSharedValue.test.ts by tracing the hitSlop type and runtime handling from
ConfigTypes.ts, then update the implementation or tests so both regular and
shared-value hitSlop behavior passes. Verify the package test suite succeeds
without regressing the existing lint, formatting, or TypeScript checks.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: efb67868-57be-440a-be3e-8b1de90b58f9
📒 Files selected for processing (5)
packages/react-native-gesture-handler/apple/RNGestureHandler.mmpackages/react-native-gesture-handler/src/handlers/hitSlop.tspackages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.tspackages/react-native-gesture-handler/src/v3/hooks/utils/reanimatedUtils.tspackages/react-native-gesture-handler/src/v3/types/ConfigTypes.ts
💤 Files with no reviewable changes (1)
- packages/react-native-gesture-handler/apple/RNGestureHandler.mm
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/react-native-gesture-handler/src/v3/hooks/utils/configUtils.ts
- packages/react-native-gesture-handler/src/v3/hooks/utils/reanimatedUtils.ts
- packages/react-native-gesture-handler/src/handlers/hitSlop.ts
| enabled?: boolean | undefined; | ||
| shouldCancelWhenOutside?: boolean | undefined; | ||
| hitSlop?: HitSlop | undefined; | ||
| hitSlop?: HitSlop | CanonicalHitSlop | undefined; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
yarn lint:js
yarn format:js
yarn ts-check
(cd packages/react-native-gesture-handler && yarn test)Repository: software-mansion/react-native-gesture-handler
Length of output: 50401
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- root scripts ---'
node - <<'JS'
const p = require('./package.json');
for (const [k, v] of Object.entries(p.scripts || {})) {
if (/^(lint:js|format:js|ts-check|test)/.test(k)) console.log(`${k}: ${v}`);
}
JS
printf '%s\n' '--- package scripts ---'
node - <<'JS'
const p = require('./packages/react-native-gesture-handler/package.json');
for (const [k, v] of Object.entries(p.scripts || {})) {
if (/^(lint:js|format:js|ts-check|test)/.test(k)) console.log(`${k}: ${v}`);
}
JS
printf '%s\n' '--- combined-command tail ---'
log=/tmp/coderabbit-shell-logs/shell-output-WkHJhD
if [ -f "$log" ]; then
tail -n 120 "$log"
else
printf '%s\n' 'Log not available'
fi
printf '%s\n' '--- repository status summary ---'
git status --short
git diff --statRepository: software-mansion/react-native-gesture-handler
Length of output: 10604
🏁 Script executed:
#!/usr/bin/env bash
set -u
printf '%s\n' '--- root scripts and workspace configuration ---'
node - <<'JS'
const p = require('./package.json');
console.log(JSON.stringify({
scripts: p.scripts || {},
packageManager: p.packageManager,
workspaces: p.workspaces
}, null, 2));
JS
printf '%s\n' '--- target type and related symbols ---'
cat -n packages/react-native-gesture-handler/src/v3/types/ConfigTypes.ts | sed -n '1,105p'
rg -n "normalizeHitSlop|expectReported|bindSharedValues|mockUpdateGestureHandlerConfig|HitSlop" \
packages/react-native-gesture-handler/src/__tests__/hitSlop.test.ts \
packages/react-native-gesture-handler/src/__tests__/hitSlopSharedValue.test.ts \
packages/react-native-gesture-handler/src/handlers/hitSlop.ts
printf '%s\n' '--- recent commit summary ---'
git log --oneline -3
git show --stat --oneline HEAD
git diff HEAD^ HEAD -- packages/react-native-gesture-handler/src/v3/types/ConfigTypes.ts \
packages/react-native-gesture-handler/src/handlers/hitSlop.ts \
packages/react-native-gesture-handler/src/__tests__/hitSlop.test.ts \
packages/react-native-gesture-handler/src/__tests__/hitSlopSharedValue.test.ts
printf '%s\n' '--- validation-stage markers from the prior log ---'
log=/tmp/coderabbit-shell-logs/shell-output-WkHJhD
if [ -f "$log" ]; then
rg -n "tsc|ts-check|Test Suites:|Tests:|FAIL |PASS |Done in|error TS|error Command|Exit code|Command failed" "$log" | tail -n 100
fiRepository: software-mansion/react-native-gesture-handler
Length of output: 13910
Resolve the failing Jest tests before merge.
yarn lint:js, yarn format:js, and yarn ts-check pass. Package yarn test fails in hitSlop.test.ts and hitSlopSharedValue.test.ts.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/react-native-gesture-handler/src/v3/types/ConfigTypes.ts` at line
77, Resolve the failing Jest cases in hitSlop.test.ts and
hitSlopSharedValue.test.ts by tracing the hitSlop type and runtime handling from
ConfigTypes.ts, then update the implementation or tests so both regular and
shared-value hitSlop behavior passes. Verify the package test suite succeeds
without regressing the existing lint, formatting, or TypeScript checks.
Source: Coding guidelines
Description
This moves the parsing and validation of
hitSlopinto 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