Reanimated

[thomasttvo]

Reanimated implementation is here and now with almost all features preserved. Some features like pin onPress/onLongPress need to be stripped due to time constraint.

---

Note

Migrates the zoomable view to a Reanimated + Gesture Handler implementation with worklet-driven callbacks and smoother animations; updates the example app and tooling to support the new stack.

• Core: Replaces legacy Animated/pan logic with Reanimated shared values, worklets, and Manual gesture handling; adds onTransformWorklet and onStaticPinPositionMoveWorklet, preserves double-tap/pinch/shift behavior, and refines static pin updates
• Example: Refactors to Reanimated (marker scaling via animated style), adds long-press alert and optional Modal wrapper, removes old zoomAnimatedValue
• Tooling: Adds Reanimated Babel plugin; updates Metro config to alias src and avoid peer duplicates; bumps example to Expo 54/React 19/RN 0.81 and adds react-native-reanimated/gesture-handler/worklets; minor style/tsconfig tweaks

^{Written by Cursor Bugbot for commit 490ae19. Configure here.}

[elliottkember]

This is looking really great — amazing work. The performance gains from things like worklets will be out of this world.

We may need to make some minor changes in our app to implement this but they should be small fry compared to these changes.

[elliottkember]

This will probably be the tricky part in our internal implementation of this. I've never truly liked the way this component takes an animated value as a prop, this is how we ended up with sheet zoom contexts where other components need to know about zoom as it happens and we needed an HOC with the animated zoom value.

It seems like an imperative handle is probably the way to do this, plus maybe some built-in components for handling interpolations — or perhaps just a context provided by this component to its children.

[elliottkember]

Yeah these kind of props sort of break the whole magic of reanimated and animated values huh?

Do we use this, or can we just subscribe to the animations and react with animated components?

I think this will maybe just be a huge major version release with lots of breaking changes that remove footguns

[cursor]

Comment @cursor review or bugbot run to trigger another review on this PR

[thomasttvo]

bugbot run

[cursor]

✅ Bugbot reviewed your changes and found no bugs!

---

Comment @cursor review or bugbot run to trigger another review on this PR

[elliottkember]

I noticed that the example and base repos have different versions of reanimated. This is a good thing, I think — it shows that the library works on both versions. But when I went to refactor runOnJs to scheduleOnRN because of a deprecation warning in Reanimated 4, I noticed that it's only deprecated in the example app.

Might be worth using the same version in both... or maybe even having two example apps for testing both reanimateds at once??

~/Sites/react-native-zoomable-view elliott/thomas-reanimated-runonjs • yarn why react-native-reanimated |grep Found
=> Found "react-native-reanimated@3.16.7"
~/Sites/react-native-zoomable-view elliott/thomas-reanimated-runonjs • cd example && yarn why react-native-reanimated |grep Found
=> Found "react-native-reanimated@4.1.6"

[elliottkember]

Clever! I think this should exhibit the same behaviour as the react-navigation / react-native-screens pull to close, but I might add the routing library and some formSheet / pageSheet routes just to test the theory

[elliottkember]

🥳 Looks good with expo-router, which uses native-stack under the hood!

[thomasttvo]

@claude please review this PR

[thomasttvo]

@claude review

[claude]

🔴 The PR uses delete doubleTapFirstTapReleaseTimestamp.value and delete doubleTapFirstTap.value (lines 659, 661, 673), but SharedValue .value is a getter/setter, not a plain own property — delete silently fails (or throws in strict mode), leaving stale timestamps that cause subsequent taps to incorrectly enter the double-tap branch. Replace all three delete sharedValue.value calls with sharedValue.value = undefined.

Extended reasoning...

What the bug is and how it manifests

The PR migrates double-tap state tracking from useRef to useSharedValue. Two shared values — doubleTapFirstTapReleaseTimestamp (declared as useSharedValue<number | undefined>(undefined)) and doubleTapFirstTap — are reset after a double tap is detected using the delete operator: delete doubleTapFirstTapReleaseTimestamp.value (line 659), delete doubleTapFirstTap.value (line 661), and again delete doubleTapFirstTapReleaseTimestamp.value inside the setTimeout callback (line 673). The original code used useRef where delete ref.current is valid, but this semantic does not carry over to Reanimated SharedValues.

The specific code path that triggers it

In _resolveAndHandleTap, when a double tap is detected, the code enters the first branch and calls delete doubleTapFirstTapReleaseTimestamp.value and delete doubleTapFirstTap.value to reset state. When a single tap times out, the setTimeout callback similarly calls delete doubleTapFirstTapReleaseTimestamp.value. In both cases the intent is to clear these shared values back to undefined.

Why existing code doesn't prevent it

Reanimated SharedValues expose .value as a getter/setter defined via Object.defineProperty on the SharedValue wrapper object — not as a plain own configurable data property. In the Reanimated v3 JSI-backed implementation, this property is non-configurable. The JavaScript delete operator on a non-configurable property silently returns false in sloppy mode and throws a TypeError in strict mode. The project's tsconfig.json has strict: true, which means every double-tap detection or single-tap timeout will throw a TypeError: Cannot delete property 'value' at runtime, crashing the gesture handler. Even in environments where delete doesn't throw (e.g., if the property were configurable), removing the getter/setter would break Reanimated's native binding for any subsequent assignment.

What the impact would be

In strict mode (confirmed by tsconfig): every double-tap crashes at line 659. Every single-tap that times out crashes at line 673. In non-strict JSI environments: delete silently fails, leaving doubleTapFirstTapReleaseTimestamp.value set to the just-completed double-tap's timestamp. The next tap within doubleTapDelay ms will satisfy now - stale < doubleTapDelay and incorrectly trigger another double-tap with stale doubleTapFirstTap.value position data.

How to fix it

Replace all three delete calls with direct assignment to undefined, which is the idiomatic Reanimated pattern and matches the declared type useSharedValue<number | undefined>(undefined):

- delete doubleTapFirstTapReleaseTimestamp.value;
+ doubleTapFirstTapReleaseTimestamp.value = undefined;
- delete doubleTapFirstTap.value;
+ doubleTapFirstTap.value = undefined;

This is consistent with how undefined is used as the initial value and how other SharedValues in the codebase are reset (e.g., line 564 uses direct assignment).

Step-by-step proof

1. User performs a double tap; _resolveAndHandleTap enters the double-tap branch.
2. Code reaches delete doubleTapFirstTapReleaseTimestamp.value (line 659).
3. doubleTapFirstTapReleaseTimestamp.value is a getter/setter on a Reanimated SharedValue — it is non-configurable.
4. In strict mode: TypeError is thrown immediately; the double-tap handler crashes and _handleDoubleTap is never called.
5. In non-strict mode: delete returns false, doubleTapFirstTapReleaseTimestamp.value still holds the old timestamp (e.g., T=1000).
6. User taps again at T=1200 (within doubleTapDelay=300ms): now - 1000 = 200 < 300 is true.
7. Code incorrectly enters the double-tap branch again, using the stale doubleTapFirstTap.value position.

[claude]

🔴 The withTiming completion callback in publicZoomTo has the directive 'worlet' (missing 'k') instead of 'worklet', so the Reanimated Babel plugin never transforms it into a worklet. As a result, every programmatic zoom via publicZoomTo crashes at animation completion because zoomToDestination.value and runOnJS are accessed from a non-worklet context on the UI thread. As a secondary effect, zoomToDestination.value is never cleared, corrupting all subsequent zoom-center animations. Fix: change 'worlet' to 'worklet' on line 561 of src/ReactNativeZoomableView.tsx.

Extended reasoning...

What the bug is and how it manifests

On line 561 of src/ReactNativeZoomableView.tsx, the withTiming completion callback begins with the string directive 'worlet' — a one-character typo for 'worklet'. React Native Reanimated's Babel plugin identifies worklet functions by an exact string-literal match on 'worklet'. Because the match fails silently, the callback is compiled as a plain JavaScript function rather than a Reanimated worklet.

The specific code path that triggers it

The bug fires whenever publicZoomTo (the public API that callers use to programmatically zoom to a specific level) is invoked. publicZoomTo drives the zoom animation via:

zoom.value = withTiming(newZoomLevel, zoomToAnimation, () => {
  'worlet';  // ← should be 'worklet'
  zoomToDestination.value = undefined;
  runOnJS(onZoomEnd)(undefined, _getZoomableViewEventObject());
});

When the animation completes, Reanimated invokes this callback on the UI thread. Because the callback was not transformed into a worklet, accessing zoomToDestination.value (a SharedValue) and calling runOnJS both throw at runtime.

Why existing code does not prevent it

The Reanimated Babel plugin performs static analysis at build time and emits no warning when a callback lacks the 'worklet' directive — it simply does not transform it. TypeScript type-checking also provides no protection here; the withTiming callback type accepts any function. Compare with lines 685–688 in the same file, where another withTiming callback is correctly annotated with 'worklet' — this confirms that withTiming callbacks are not auto-workletized and must be explicitly marked.

Impact

Every call to publicZoomTo will produce a runtime crash on animation completion. Additionally, because zoomToDestination.value is never reset to undefined, the useAnimatedReaction block that watches zoomToDestination and adapts offsets during a zoom-to animation will continue firing on every subsequent change to zoom.value, silently corrupting pan and zoom-center behavior for the lifetime of the component.

How to fix it

Change 'worlet' to 'worklet' on line 561. The single-character fix ensures the Reanimated Babel plugin transforms the callback into a proper worklet, allowing both the SharedValue write and the runOnJS call to execute correctly on the UI thread.

Step-by-step proof

1. User calls ref.zoomTo(2), which invokes publicZoomTo(2).
2. publicZoomTo sets zoom.value = withTiming(2, zoomToAnimation, callback).
3. After ~300 ms the animation finishes; Reanimated invokes callback on the UI thread.
4. callback was not transformed (typo), so it is a regular JS closure running on the UI thread.
5. zoomToDestination.value = undefined — accessing .value on a SharedValue outside a worklet throws: "Reading from value during single frame update is not allowed".
6. The error surfaces as an unhandled native exception, crashing the component (or the app if not caught).
7. Because step 5 threw, zoomToDestination.value remains set to its last zoom target, and runOnJS(onZoomEnd)(...) is never called — the onZoomEnd callback is silently dropped.

[claude]

🔴 In calcGestureTouchDistance, dy is computed using touches[0].x - touches[1].x instead of touches[0].y - touches[1].y — a copy-paste error from the line above. This means the Euclidean distance is calculated as sqrt(dx² + dx²) rather than sqrt(dx² + dy²), causing purely vertical pinches to return a distance of 0 (breaking zoom entirely) and all non-horizontal pinches to report incorrect distances. Fix: change touches[0].x to touches[0].y on the dy line in src/helper/index.ts.

Extended reasoning...

Bug: Copy-paste error in calcGestureTouchDistance computes dy using X coordinates

What the bug is

In src/helper/index.ts (lines 44–48), the helper function calcGestureTouchDistance computes the Euclidean distance between two touch points for pinch-to-zoom. The dy (vertical delta) variable is mistakenly assigned the same expression as dx (horizontal delta):

const dx = Math.abs(touches[0].x - touches[1].x);
const dy = Math.abs(touches[0].x - touches[1].x); // BUG: should use .y
return Math.sqrt(dx * dx + dy * dy);

This means the function returns sqrt(dx² + dx²) instead of the correct sqrt(dx² + dy²).

How it manifests

The function is called inside the Gesture.Manual() onTouchesMove handler to track pinch distance changes. When the computed distance is wrong, the scale delta passed to the zoom logic is wrong, producing incorrect zoom behavior.

Why existing code doesn't prevent it

This is a silent logic error — no type-check or runtime assertion catches it. The function still returns a number; it just returns the wrong number. The TypeScript compiler cannot detect that two variables intended to represent orthogonal axes are using the same coordinate.

Impact

• Purely vertical pinch: dx = 0, so dy = 0 as well (since it's also using x). Distance = sqrt(0) = 0. The zoom scale delta becomes 0 or undefined, effectively disabling zoom for vertical pinch gestures entirely.
• Diagonal pinch: Distance is over-estimated (e.g., a 3-4-5 triangle gives sqrt(3²+3²) ≈ 4.24 instead of sqrt(3²+4²) = 5), causing the wrong zoom scale to be applied throughout the gesture.
• Horizontal pinch only: This is the sole case that works correctly, since dy contributes 0 to the true distance anyway.

Step-by-step proof

Suppose finger 1 is at (100, 50) and finger 2 is at (100, 150) — a perfectly vertical pinch spanning 100px.

• Correct: dx = |100-100| = 0, dy = |50-150| = 100, distance = sqrt(0 + 10000) = 100
• Buggy: dx = |100-100| = 0, dy = |100-100| = 0 (x used for both), distance = sqrt(0 + 0) = 0

A distance of 0 on every onTouchesMove event means the scale ratio is computed as 0 / previousDistance, collapsing zoom to 0 or causing a divide-by-zero, breaking the gesture entirely.

Fix

Change line 45 from:

const dy = Math.abs(touches[0].x - touches[1].x);

to:

const dy = Math.abs(touches[0].y - touches[1].y);

[claude]

🟡 In the Manual gesture handler's onTouchesDown callback, stateManager.activate() is called before stateManager.begin(), reversing the documented RNGH state machine order (UNDETERMINED → BEGAN via begin(), then BEGAN → ACTIVE via activate()). While RNGH's activate(force=true) implementation does permit a direct UNDETERMINED → ACTIVE transition so the gesture functions correctly in practice, the subsequent begin() call becomes dead code (since the gesture is already ACTIVE and cannot regress to BEGAN). The fix is a one-line swap: call stateManager.begin() first, then stateManager.activate().

Extended reasoning...

What the bug is and how it manifests

In src/ReactNativeZoomableView.tsx (lines ~895-896), the onTouchesDown handler for the Gesture.Manual() gesture calls stateManager.activate() before stateManager.begin(). The documented RNGH Manual gesture state machine requires: UNDETERMINED → BEGAN (via begin()) → ACTIVE (via activate()). The PR has these two lines in the wrong order.

The specific code path

The code in onTouchesDown calls stateManager.activate() first and stateManager.begin() second, when it should be the opposite order.

Why existing code does not prevent it

RNGH's JS-side stateManager.activate() routes through setGestureHandlerState(..., State.ACTIVE) which ultimately calls the native handler.activate(force=true). With force=true, the manualActivation guard is bypassed, and the state check (state == STATE_UNDETERMINED || state == STATE_BEGAN) explicitly permits a direct UNDETERMINED → ACTIVE transition. So the gesture does reach ACTIVE state despite the reversed order. The subsequent begin() call (which attempts ACTIVE → BEGAN) is then either rejected by the state machine or is a no-op.

Impact

The PR reviewer (elliottkember) tested zoom and pinch on the simulator and confirmed it works correctly, corroborating the analysis that the reversed order does not break runtime behavior. However, the begin() call is semantically dead code. The code violates the documented API contract and all official RNGH examples, which consistently call begin() before activate().

Step-by-step proof

1. User puts finger down, onTouchesDown fires.
2. stateManager.activate() is called while state is UNDETERMINED.
3. RNGH native: activate(force=true) is called; state == STATE_UNDETERMINED passes, gesture moves to ACTIVE.
4. stateManager.begin() is called while state is already ACTIVE.
5. RNGH native: begin() attempts ACTIVE → BEGAN; this backwards transition is rejected (no-op).
6. Gesture stays in ACTIVE. Touch events continue to be delivered. Pan/zoom works.
7. Net result: correct behavior, but begin() is dead code.

How to fix it

Swap the two lines so begin() is called first (UNDETERMINED → BEGAN), then activate() (BEGAN → ACTIVE).