part I. Strip complicated but low priority features ahead of reanimated conversion

[thomasttvo]

Since we'll convert to reanimated in a later PR, it's important to strip some features to avoid a complicated migration. We can always reintroduce them in the reanimated world if we need to.

Note

Remove pan-boundary/momentum and pin animation features, simplify offsets/gestures, and update API, docs, and builds accordingly.

• Core changes:
  • Remove pan-boundary logic, momentum decay, and pin animations; simplify offset handling and gesture flow; delete applyPanBoundariesToOffset and related animation helpers.
  • Modernize code (optional chaining, JSX runtime) and streamline internal state.
• API/Props:
  • Drop bindToBorders, panBoundaryPadding, disableMomentum, and animatePin; remove pinAnim from StaticPin; methods no longer accept bindToBorders params.
  • Update typings and d.ts accordingly; add useLatestCallback hook.
• Docs/Examples/Build:
  • Revise README tables/usages and example app to reflect removed props.
  • Keep only getZoomToAnimation in animations; add package type fields in build outputs.

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

[cursor]

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

[elliottkember]

I think this should be fine, but it's a breaking change so we'll need a minor version, and a CHANGELOG.md or some kind of breaking change notice in the release. I'll be interested to see whether anybody is using these features and has trouble upgrading

[thomasttvo]

@elliottkember that's a good point. I've created this PR to create a better setup for changelog generation. We can edit the changelog (once it's generated) to explain further which features have been dropped once we publish. Additionally, we can also publish this as part of an RC version together with the reanimated stuff and ask the community for feedback before turning it into a official version

[elliottkember]

@thomasttvo Sounds like a plan. It may actually be worth making a new major version RC. Given the scope of the reanimated change I think it's justified, and will avoid anybody accidentally updating their if they use ^2 in package.json.

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude please review this PR

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[thomasttvo]

@claude review

[claude]

---

Additional findings (outside current diff — PR may have been updated during review):

• 🔴 src/ReactNativeZoomableView.tsx:1033-1060 — This PR introduces a listener leak on zoomAnim when zoomTo() is called twice in rapid succession (e.g. two fast double-taps). By promoting listenerId from a local variable to the shared class field this.zoomToListenerId, the second call silently drops the first listener ID without removing it from zoomAnim, causing it to leak permanently. When zoomAnimatedValue is an external prop outliving the component, each rapid double-tap accumulates another stale listener firing panAnim.setValue() with stale values. Fix: restore a per-invocation local variable (closed over by each animation callback) or track all in-flight IDs in a Set.

  Extended reasoning...

  What the bug is and how it manifests

  The PR replaced the local variable let listenerId = '' inside zoomTo() with the class field this.zoomToListenerId. The old local variable was correctly captured in a closure for each individual call to zoomTo(), making cleanup fully independent between invocations. The new class field is shared across all invocations, breaking that isolation when two calls overlap.

  The specific code path that triggers it

  Inside zoomTo() (around lines 1033-1075 in the current source):

  1. this.zoomToListenerId = undefined
  2. this.zoomToListenerId = this.zoomAnim.addListener(...) -> assigns 'L1'
  3. getZoomToAnimation(...).start(cb1) -- animation 1 begins

  If a second call fires before animation 1 completes:

  1. this.zoomToListenerId = undefined -- the reference to 'L1' is now gone, but 'L1' is still registered on zoomAnim
  2. this.zoomToListenerId = this.zoomAnim.addListener(...) -> assigns 'L2'
  3. getZoomToAnimation(...).start(cb2) -- animation 2 begins, stopping animation 1

  Why existing code does not prevent it

  When animation 1 is interrupted, cb1 fires with {finished: false}. At that point this.zoomToListenerId holds 'L2' (not 'L1'), so cb1 removes 'L2' and sets the field to undefined. When animation 2 completes, cb2 checks if (this.zoomToListenerId) and finds undefined, so it removes nothing. 'L1' is never removed.

  componentWillUnmount guards: if (this.zoomToListenerId) zoomAnim.removeListener(...) -- but after the race the field is undefined, so unmount cleanup also misses 'L1'.

  What the impact would be

  Every rapid double-tap permanently adds one more stale listener to zoomAnim. Each leaked listener calls panAnim.setValue() on every subsequent animation tick, computing pan offsets against stale prevScale/offsetX/offsetY values. When zoomAnimatedValue is an external prop belonging to the parent (and outliving the child), these listeners accumulate for the entire parent lifetime -- continuously triggering side effects on a dead component instance.

  Step-by-step proof

  1. User double-taps -> _handleDoubleTap calls zoomTo(step1, center). this.zoomToListenerId = 'L1'. A zoom animation starts.
  2. ~50ms later, user double-taps again -> second zoomTo(step2, center) fires. Line sets this.zoomToListenerId = undefined. 'L1' is now orphaned on zoomAnim. this.zoomToListenerId = 'L2'. Animation 2 starts, stopping animation 1.
  3. Animation 1's .start() callback fires (finished: false). this.zoomToListenerId is 'L2' -> zoomAnim.removeListener('L2') executes. this.zoomToListenerId = undefined.
  4. Animation 2 completes. this.zoomToListenerId is undefined -> the if guard is falsy -> nothing removed.
  5. 'L1' remains registered forever, calling panAnim.setValue() on every future zoomAnim tick.

  How to fix it

  Restore the local-variable approach: let listenerId: string | undefined scoped to each zoomTo() call, closed over by both the listener callback and the animation completion callback. Alternatively, track all in-flight listener IDs in a Set on the class and drain the set in componentWillUnmount.

[thomasttvo]

@claude review

[claude]

🔴 The .start() callback in _resolveAndHandleTap ignores the finished flag, so panAnim.stopAnimation() in componentWillUnmount synchronously invokes _updateStaticPin() → onStaticPinPositionChange during unmount. The clearTimeout(singleTapTimeoutId) added in f095c20 only guards the case where the timer hasn't fired; if the 200ms animation is already running when unmount occurs, stopAnimation() fires the callback immediately with {finished: false}, which is not checked. Fix: .start(({finished}) => { if (finished) this._updateStaticPin(); }).

Extended reasoning...

The incomplete fix

f095c20 added clearTimeout(this.singleTapTimeoutId) to componentWillUnmount. This correctly prevents the post-unmount scenario where the doubleTapDelay timer fires after unmount and starts a brand-new animation. However, it does not cover the race where the timer fires before unmount.

The remaining bug — step-by-step

1. User single-taps at t=0. _resolveAndHandleTap schedules singleTapTimeoutId = setTimeout(cb, doubleTapDelay=300ms).
2. At t=300ms the setTimeout fires. Inside cb, delete this.singleTapTimeoutId executes first (so singleTapTimeoutId becomes undefined). Then Animated.timing(this.panAnim, {duration: 200, useNativeDriver: true}).start(() => { this._updateStaticPin(); }) starts.
3. Component unmounts at t=350ms (50ms into the 200ms animation).
4. componentWillUnmount runs. clearTimeout(this.singleTapTimeoutId) → clearTimeout(undefined) → no-op. panAnim.stopAnimation() is called.
5. React Native Animated internals propagate the stop: ValueXY.stopAnimation() → Value.stopAnimation() → TimingAnimation.stop() → __debouncedOnEnd({finished: false}) → Parallel completion → the user's .start(cb).
6. The callback is () => { this._updateStaticPin(); } — no finished check — so _updateStaticPin() → onStaticPinPositionChange fires synchronously during componentWillUnmount.

Why existing guards do not prevent it

The clearTimeout guard only covers a pending timer. Once the timer fires and the animation starts, singleTapTimeoutId is deleted and clearTimeout becomes a no-op. The stopAnimation() call, intended to prevent a post-unmount callback, instead causes a during-unmount callback by synchronously delivering the stop signal through React Native's Animated pipeline.

Impact

If the parent's onStaticPinPositionChange handler updates state in a component that has already unmounted (e.g. the parent is also mid-transition), this produces React warnings ('Cannot update a component from inside the function body of a different component') or state-on-unmounted-component errors. Even when the parent is still mounted, receiving a position callback during a child's componentWillUnmount is semantically unexpected and violates the principle that lifecycle callbacks should not emit side effects after destruction begins. The scenario is plausible on any navigation-heavy app where a single-tap initiates a pan animation and the user navigates away within 500ms.

The fix

Change the .start() callback in _resolveAndHandleTap from:

to:

This ensures _updateStaticPin is only called when the animation completed naturally, not when it was aborted by stopAnimation().

[claude]

🔴 When zoomTo() is called while a previous zoomTo() animation is in flight, this.zoomToListenerId = undefined at the start of the method silently discards the first listener ID without calling removeListener, permanently leaking listenerA. The first animation's completion callback then fires with {finished: false}, finds the second call's listener ID in the field, removes it prematurely, and clears the field — so when the second animation completes, its listener is already gone, breaking pan centering for the second zoom. Fix: guard the reset with if (this.zoomToListenerId) { this.zoomAnim.removeListener(this.zoomToListenerId); this.zoomToListenerId = undefined; } before adding the new listener.

Extended reasoning...

What the bug is and how it manifests

This PR promoted the local listenerId variable in zoomTo() to the class field this.zoomToListenerId to support componentWillUnmount cleanup. However, the method begins by unconditionally setting this.zoomToListenerId = undefined (line 1043) before assigning the new listener ID. When zoomTo() is called a second time before the first animation completes — triggered by rapid double-taps (which always pass a zoomCenter via _handleDoubleTap) or programmatic successive calls — this reset silently orphans the first listener's ID.

The specific code path

1. zoomTo(levelA, centerA) runs: this.zoomToListenerId = undefined (no previous listener, harmless), then this.zoomToListenerId = this.zoomAnim.addListener(...) stores listenerA's ID. getZoomToAnimation(...).start(callbackA) begins animationA.
2. Before animationA finishes, zoomTo(levelB, centerB) runs: this.zoomToListenerId = undefined drops listenerA's ID without calling removeListener — listenerA is now permanently leaked. Then this.zoomToListenerId = this.zoomAnim.addListener(...) stores listenerB's ID. getZoomToAnimation(...).start(callbackB) begins animationB on the same zoomAnim, which stops animationA.
3. AnimationA's callbackA fires with {finished: false}. It checks if (this.zoomToListenerId) — true, because it now holds listenerB's ID. It calls this.zoomAnim.removeListener(this.zoomToListenerId) (removing listenerB) and sets this.zoomToListenerId = undefined.
4. AnimationB completes. callbackB fires, checks if (this.zoomToListenerId) — false. No cleanup occurs. ListenerB is already gone, so the panAnim tracking of the zoom center during animationB stopped prematurely.

Why existing code doesn't prevent it

The original code used a local let listenerId = '' captured in each closure independently, so the two callbacks could not interfere. Promoting to a class field breaks this isolation — there is only one slot for one ID, so the second call clobbers the first before the first has a chance to clean up after itself.

Impact

(a) Every interrupted zoomTo() call permanently leaks a zoomAnim listener. Since the listener fires on every zoomAnim change for the lifetime of the animated value (which may be an externally provided zoomAnimatedValue prop that outlives the component), this becomes a growing memory and CPU leak. (b) For the second zoomTo() call with a zoomCenter, listenerB is removed mid-animation, so panAnim stops tracking the zoom center, causing incorrect visual centering during that zoom.

Step-by-step proof

1. User performs a rapid double-tap. _handleDoubleTap calls this.zoomTo(nextZoomStep, zoomPositionCoordinates) — call A. this.zoomToListenerId = listenerA's ID. AnimationA starts.
2. Before animationA finishes (e.g., 150ms into a 300ms animation), user double-taps again. this.zoomTo(initialZoom, zoomPositionCoordinates) — call B. this.zoomToListenerId = undefined orphans listenerA. this.zoomToListenerId = listenerB's ID. AnimationB starts, stopping A.
3. AnimationA's callback fires (finished: false). this.zoomToListenerId is listenerB's ID, so listenerB is removed and the field cleared.
4. AnimationB completes. this.zoomToListenerId is undefined, so listenerB cleanup is skipped (it was already removed).
5. ListenerA is now permanently attached to zoomAnim, firing on every subsequent zoom change. The second zoom animation produced incorrect pan centering because its listener was removed partway through.

How to fix it

Before assigning a new listener, remove the old one if it exists:

if (this.zoomToListenerId) {
  this.zoomAnim.removeListener(this.zoomToListenerId);
  this.zoomToListenerId = undefined;
}
this.zoomToListenerId = this.zoomAnim.addListener(...);

This restores the isolation that the original local-variable approach provided, without losing the unmount-cleanup capability that the class field was introduced for.

[claude]

The fix at lines 1046-1048 correctly prevents the permanent leak of listenerA — a genuine improvement. However, problem (b) from the original comment (callbackA prematurely removing listenerB) is still present.

Trace with zoomCenter provided in both rapid calls:

1. Call A: guard is a no-op, add listenerA → this.zoomToListenerId = listenerA_ID, animationA starts.
2. Call B (mid-animation): guard removes listenerA ✅, this.zoomToListenerId = undefined, add listenerB → this.zoomToListenerId = listenerB_ID, animationB starts (which stops animationA).
3. CallbackA fires ({finished: false}): if (this.zoomToListenerId) → true (holds listenerB_ID) → removeListener(listenerB_ID), this.zoomToListenerId = undefined ❌ — listenerB is gone mid-animation.
4. AnimationB completes: this.zoomToListenerId is already undefined → no cleanup, but pan centering was silently broken from step 3 onward.

The root cause is that the completion callback still reads this.zoomToListenerId from the shared class field rather than from a captured local. The original local-variable approach was safe because each closure captured its own listenerId. Promoting to a class field but keeping the callback reading the field loses that isolation.

The minimal complete fix is to capture the listener ID in the closure after assigning it, and compare before clearing:

if (this.zoomToListenerId) {
  this.zoomAnim.removeListener(this.zoomToListenerId);
  this.zoomToListenerId = undefined;
}
if (zoomCenter) {
  this.zoomToListenerId = this.zoomAnim.addListener(/* ... */);
}
const capturedId = this.zoomToListenerId; // capture the ID for *this* call
getZoomToAnimation(this.zoomAnim, newZoomLevel).start(() => {
  if (capturedId) {
    this.zoomAnim.removeListener(capturedId);
    if (this.zoomToListenerId === capturedId) {
      this.zoomToListenerId = undefined; // only clear if still ours
    }
  }
});

This way callbackA only removes listenerA (already gone — no-op) and never touches the class field that now holds listenerB_ID.

[claude]

🟣 The README methods table documents zoomTo() and zoomBy() as returning Promise<bool> and moveTo()/moveBy() as returning Promise<void>, but none of these methods return Promises — they return plain booleans or void. This PR explicitly rewrote the methods table rows (removing bindToBorders = true params) while leaving the stale Promise<bool>/Promise<void> return types in place, a missed opportunity to correct this pre-existing doc error.

Extended reasoning...

What the bug is

The README Methods table (lines 207–212) documents zoomTo() and zoomBy() as returning Promise<bool>, and moveTo()/moveBy() as returning Promise<void>. The actual implementations return plain booleans and void — no Promises anywhere.

The specific code paths

• zoomTo(): explicitly returns false (three early returns) or true at the end — plain booleans
• zoomBy(): return this.zoomTo(...) — returns whatever zoomTo returns (boolean)
• moveTo(): no return statement — void
• moveBy(): no return statement — void

None of these methods call new Promise, async/await, or wrap anything in a Promise.

Why existing code does not prevent it

This is a pure documentation bug. TypeScript only enforces types against the implementation, not README prose. Nothing in the build or CI process validates whether the README's "expected return" column matches the actual method signatures.

Impact

A developer who reads the docs and writes await zoomableViewRef.current.zoomTo(1.5) expecting the await to resolve after the animation completes will be silently surprised: await on a non-thenable resolves immediately with the value itself (i.e., true). Any post-animation logic placed after such an await runs before the animation finishes — a silent behavioral bug for any consumer writing async animation-sequencing code.

Step-by-step proof

1. Developer reads README: zoomTo returns Promise<bool>
2. Developer writes: await zoomableViewRef.current.zoomTo(1.5); showNextPanel();
3. zoomTo() starts a getZoomToAnimation(...).start() and immediately returns true (a plain boolean)
4. await true resolves in the same microtask — showNextPanel() is called before the 300ms animation completes
5. The animation is still running; the UI shows both the zoom animation and the next panel simultaneously

Pre-existing nature and missed opportunity

This documentation error predates this PR. However, this PR explicitly modified these exact table rows — removing bindToBorders = true from the params column — while leaving the stale Promise<bool>/Promise<void> return types unchanged. Since the rows were already being edited, fixing the return types would have been a one-column change per row.

[claude]

🔴 This PR's new componentWillUnmount calls this.panAnim.stopAnimation() and this.zoomAnim.stopAnimation() unconditionally, but the constructor replaces these with externally-owned values when zoomAnimatedValue or panAnimatedValueXY props are provided. If a parent passes these props and independently animates those same values, unmounting the ZoomableView abruptly stops the parent's animation mid-flight. The fix is to guard the calls: if (!this.props.zoomAnimatedValue) this.zoomAnim.stopAnimation(); and similarly for panAnim.

Extended reasoning...

What the bug is and how it manifests

This PR introduces this.panAnim.stopAnimation() and this.zoomAnim.stopAnimation() in componentWillUnmount to stop in-flight animations and prevent post-unmount callbacks. However, the constructor contains:

if (this.props.zoomAnimatedValue) this.zoomAnim = this.props.zoomAnimatedValue;
if (this.props.panAnimatedValueXY) this.panAnim = this.props.panAnimatedValueXY;

This means this.zoomAnim and this.panAnim may refer to Animated values that belong to and outlive the parent component. The unconditional stopAnimation() calls in componentWillUnmount will interrupt any animation the parent is running on those shared values.

The specific code path that triggers it

1. Parent creates zoomAnimatedValue = new Animated.Value(1) and passes it as a prop.
2. Parent also calls Animated.timing(zoomAnimatedValue, { toValue: 2, duration: 500 }).start() to animate a sibling overlay component.
3. ZoomableView unmounts (navigation transition). componentWillUnmount fires and calls this.zoomAnim.stopAnimation(), which is the parent-owned zoomAnimatedValue, cancelling the parent animation mid-flight.
4. The sibling overlay freezes at whatever zoom value it had reached, even though the parent expects the animation to complete.

Why existing code does not prevent it

The stopAnimation calls were added by this PR with no guard. The only consideration in the PR was preventing post-unmount callbacks from internally-initiated animations, but the implementation is broader than needed and also stops externally-provided animation values unconditionally. Before this PR, componentWillUnmount did not call stopAnimation() at all, so this is a regression in the external-value case.

What the impact would be

Any parent that passes zoomAnimatedValue or panAnimatedValueXY and concurrently drives animations on those values from outside the component will have those animations silently terminated on unmount. The caller receives no error or warning; the animation simply stops.

Addressing the refutations

Two verifiers argue the scenario is implausible because the typical usage (App.tsx) is read-only: the parent derives scale = Animated.divide(1, zoomAnimatedValue) but never calls Animated.timing on it directly. This is a fair characterization of the canonical pattern. However: (1) the API does not prohibit or document against parent-driven animations on these values; (2) the fix is a one-line guard per value and adds no cost to typical usage; (3) the PR intent is to stop internally-initiated animations to suppress post-unmount callbacks, and that intent is better served by a guarded call than an unconditional one. The refutation that the constructor already halts pre-existing animations via setValue is true, but irrelevant here: setValue happens at mount, whereas the hazard is animations started by the parent after mount and still in flight at unmount.

How to fix it

In componentWillUnmount, guard both calls:

if (!this.props.panAnimatedValueXY) this.panAnim.stopAnimation();
if (!this.props.zoomAnimatedValue) this.zoomAnim.stopAnimation();

This preserves the PR intent (cancelling internal animations) while respecting external ownership of animated values.