fix: evaluate mount position from JS when animated reactions never fire - #2720
Open
icantcodefyi wants to merge 1 commit into
Open
fix: evaluate mount position from JS when animated reactions never fire#2720icantcodefyi wants to merge 1 commit into
icantcodefyi wants to merge 1 commit into
Conversation
On Reanimated v4, every useAnimatedReaction attached to a sheet instance can fail to fire when the instance mounts while the JS thread is busy. Derived values (detents, isLayoutCalculated) still compute correctly, but the reaction that evaluates the initial position never runs and the sheet stays parked off-screen at the container height, while onChange and provided animatedIndex/animatedPosition values never update either. Two changes: 1. Seed containerHeight alongside rawContainerHeight in the same layoutState.modify at the point of measurement, so initial layout does not depend on the raw->container reaction firing (mirrors gorhom#2689 by @christian-apollo). 2. Re-dispatch the mount position evaluation from the JS side via runOnUI until didAnimateOnMount flips (100ms interval, max 20 attempts), skipping while an animation is already running. This runs the exact evaluatePosition the reaction would have run; with healthy reactions the first check observes the mount already handled and does nothing.
Closed
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes #2721 (and the stale-closed #2690; an earlier report #2719 was auto-closed for not following the issue form): on Reanimated 4, every
useAnimatedReactionattached to a sheet instance can fail to register when the instance mounts while the JS thread is busy. Derived values (detents,isLayoutCalculated) still compute, but the reaction that runs the mount position evaluation never fires — the sheet mounts parked off-screen at container height,onChangenever fires, and providedanimatedIndex/animatedPositionvalues are never written. Reproduced in production on mid-tier Android (details and Sentry field evidence in #2719).Changes
1. Seed
containerHeightat the point of measurement (BottomSheetHostingContainer): writecontainerHeightalongsiderawContainerHeightin the samelayoutState.modify, so initial layout state doesn't depend on the raw→container reaction firing. This incorporates #2689 — credit to @christian-apollo; happy to rebase if that lands first.verticalInsethandling matches the reaction it replaces (modal subtracts insets, non-modal doesn't; modals passshouldCalculateHeight={false}so in practice this path serves non-modal sheets).2. JS-driven mount evaluation fallback (
BottomSheet): a mount effect re-dispatchesevaluatePosition(ANIMATION_SOURCE.MOUNT)viarunOnUIevery 100ms (max 20 attempts) untildidAnimateOnMountflips, skipping while an animation is alreadyRUNNING(so keyboard/mount animations are never interrupted). It runs the exact evaluation the reaction would have run — same guards, same branches — so semantics are unchanged. When reactions are healthy, the first check observesdidAnimateOnMount === trueand the effect retires without dispatching anything.Both parts are needed: without (1),
evaluatePosition/snapToIndexearly-exit on "layout not ready" forever; without (2), the sheet still mounts invisible with perfectly correct layout state because nothing ever moves it.Behavior when healthy
didAnimateOnMounttrue → cleanup, zero UI dispatches.animateOnMount(mount animation running) →RUNNINGguard skips until it completes, thendidAnimateOnMountis true → no-op.index={-1}) → evaluation resolves the closed position exactly as the reaction path would.Testing
Shipped as a patch-package in production (Android-heavy language-learning app): the mount-invisible reports stopped; an independent
measureInWindow-based watchdog we run alongside confirms sheets now position on mount even on the devices that previously reproduced it.