refactor!: API review — group isolation, ref leak, and a three-stage cleanup

[arekkubaczkowski]

Targets the swmansion 0.16.2 branch (#39), not main — stack this after it.

A full read-through of the API surface (public and internal), written up in API-REVIEW.md, then applied in three stages. Nine bugs, ten API inconsistencies, eight internal ones, six dead symbols.

Stage 1 — bugs, no API change

Group isolation was broken in three places. BottomSheetManagerProvider promises independent groups, but the store kept one global stackOrder and three operations walked it without filtering by groupId:

• applyModeToTopSheet — mode: 'switch'/'replace' in group B hid or closed a sheet in group A
• getTopSheetId in finishClosing — closing in group B auto-restored a hidden sheet from group A
• getSheetBelowId in startClosing — same

The coordinator, useSheetRenderData, closeAllAnimated and clearGroup all filtered correctly, so the invariant was known and just applied inconsistently. Rather than adding a fourth, fifth and sixth filter, the stack is now keyed by group (stackOrderByGroup): an operation cannot reach another group's sheets because it never holds their stack. CLAUDE.md gains an explicit warning that flattening the map inside a store action or selector reintroduces exactly this bug.

Ref leak. open() wrote the adapter ref into the module-global map before the store could reject the call. cleanupSheetRef only runs from QueueItem's unmount, so a rejected open leaked an entry nothing could reclaim — once per call, since inline IDs are random. The ref is now registered only on success, which is what forced open() to report its outcome.

Conditional hook call. useOnBeforeClose threw before useEvent and useEffect, so losing the context mid-unmount dropped the hook count from 4 to 2 and React reported "rendered fewer hooks than expected" instead of the real problem.

Sheets stuck in 'closing'. The coordinator had two strategies in one switch: expand re-read the ref inside rAF, close used a value captured at subscription time. If the adapter had not mounted yet — realistic for a portal, whose content must teleport first — ref?.close() was a silent no-op, nothing called handleClosed(), and the sheet sat in 'closing' forever while blocking every later open in its group. Both transitions now go through one retry helper that re-reads the ref and re-checks the status each frame, with a dev warning when it gives up.

Plus: a ref mutated inside a Zustand selector (useSheetScaleDepth), indexOf over the array being iterated in closeAllAnimated, and requestClose returning true for sheets it never touched.

Stage 2 — behavioural consistency

Backdrops that snapped. animatedIndex was driven continuously by gorhom and swmansion but discretely by the other three, so their backdrops jumped to full opacity on the first frame while the sheet was still animating. CustomModalAdapter was the clearest case — it animates progress over 300 ms and set animatedIndex discretely in the same expand(). This is the same symptom reported for swmansion in #39, baked into the adapters instead of the backdrop.

Each now follows its own animation: CustomModalAdapter derives from the progress it already has, ReactNativeModalAdapter fades over animationInTiming/animationOutTiming, ActionsSheetAdapter springs with the sheet's own spring config (openAnimationConfig is a WithSpringConfig — there is no duration to borrow).

Typed adapters. [key: string]: unknown in two adapters turned out to be a consequence of the libraries not being devDependencies, not an oversight. Added both (as gorhom and swmansion already were) and typed the adapters against the real prop types. ModalProps needed Partial<> — react-native-modal declares most props required and fills them from defaultProps.

Adapter authors get the full toolkit. Every built-in adapter calls useSetBackdrop to suppress the shared backdrop; it was never exported, so a custom adapter written against docs/custom-adapters.md could not match them. Now exported, and documented.

Also: open() returns string | null with a dev warning naming the rejection reason, close() returns Promise<boolean> (it already existed on requestClose and closeAll — the three main close() calls just dropped it), useBottomSheetStatus accepts BottomSheetPortalId for completion, and six dead symbols are gone.

Stage 3 — cleanup (breaking)

• clear() → destroyAll(). The old name read as tidying up while bypassing onBeforeClose and skipping the exit animation.
• isOpen now means open, not "open or opening". Added isOpening / isClosing / isVisible.
• useBottomSheetStore marked @internal; public BottomSheetState narrowed to the stable fields.
• Test helpers moved to a /testing subpath, with one resetBottomSheetRegistries() that cannot go stale as registries are added.
• Removed all eight deprecated aliases.
• Local useEvent → useStableCallback (it collided with reanimated's unrelated useEvent, and the swmansion adapter imports both); bottomSheet.store.ts re-export layer collapsed into ./store.

Verification

yarn typecheck, yarn lint and yarn prepare (bob + React Compiler at panicThreshold: 'all_errors') pass. Confirmed the /testing subpath builds and that the removed symbols are gone from the published index.d.ts.

Nothing was run on a device. The repo still has no test suite (jest reports 0 matches), so none of these fixes is covered by a test. The changes most worth exercising on hardware are the three adapters whose backdrop timing changed and the coordinator's close path — both alter animation behaviour that static analysis cannot confirm.

Not fixed

The native detent cap derives from getLocationInWindow / convert(to: window), which account for transforms. Sheets render inside ScaleWrapper, so a sheet scaled by another above it may recompute its cap and twitch. Left alone — separate concern from anything here, and it touches the core scale animation.

---

Generated by Claude Code