From 4815a73836885c989ce43cdd45b639fc90bfc0fe Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 18:49:55 +0000 Subject: [PATCH 01/13] docs: add full API review (public + internal) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-through of the whole surface — public exports, store, coordinator, registries, contexts, hooks and all five adapters — looking for inconsistencies and maintenance hazards. Nine bugs found, the notable ones being group isolation (three store operations reach into the global stackOrder without filtering by groupId, so 'switch'/'replace' in one group can hide or close a sheet in another), a ref leak in sheetRefsMap whenever open() is silently rejected, and a conditional hook call in useOnBeforeClose. Also documents API-level inconsistencies: close() drops the interceptor result that closeAll() returns, clear() bypasses onBeforeClose despite its innocuous name, animatedIndex is driven continuously by two adapters and binarily by three (so their backdrops snap instead of fading), and useSetBackdrop is used by every built-in adapter but never exported for custom ones. --- API-REVIEW.md | 436 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 436 insertions(+) create mode 100644 API-REVIEW.md diff --git a/API-REVIEW.md b/API-REVIEW.md new file mode 100644 index 0000000..4fa0148 --- /dev/null +++ b/API-REVIEW.md @@ -0,0 +1,436 @@ +# Przegląd API — `react-native-bottom-sheet-stack` + +Przegląd całej powierzchni API (publicznej i wewnętrznej) pod kątem spójności, +łatwości użycia i utrzymania. Stan na commit bazowy: bump swmansion do 0.16.2. + +Wszystkie ustalenia pochodzą z czytania kodu. **Nic nie zostało uruchomione na +urządzeniu ani pokryte testem** — repo nie ma suite'u testowego. Tam, gdzie +odróżnienie „bug” od „zamierzone” wymaga uruchomienia, jest to zaznaczone. + +Numeracja: `B*` = bug, `P*` = publiczne API, `W*` = wewnętrzne, `M*` = martwy kod. + +--- + +## 1. Bugi + +### B1. Izolacja grup jest złamana w trzech miejscach — **priorytet 1** + +`BottomSheetManagerProvider` obiecuje niezależne grupy („Each group has its own +stackOrder”). Store trzyma jednak **jeden globalny `stackOrder`**, a trzy operacje +sięgają do niego bez filtrowania po `groupId`: + +| Miejsce | Kod | Skutek | +|---|---|---| +| `store.ts:37` | `applyModeToTopSheet(sheetsById, state.stackOrder, mode)` | `mode: 'switch'`/`'replace'` w grupie B ukrywa/zamyka sheet z grupy A | +| `store.ts:118` | `getTopSheetId(newStackOrder)` | Po zamknięciu sheeta w grupie B auto-przywracany jest ukryty sheet z grupy A | +| `store.ts:92` | `getSheetBelowId(state.stackOrder, id)` | `startClosing` przywraca sheet „poniżej” z innej grupy | + +Dla porównania — te same operacje **filtrują** poprawnie w `initBottomSheetCoordinator`, +`useSheetRenderData`, `closeAllAnimated` i `clearGroup`. Czyli inwariant jest znany, +tylko niekonsekwentnie stosowany. + +**Naprawa:** przekazać `groupId` do trzech helperów i filtrować, albo — czyściej — +trzymać `stackOrder` per grupa: `stackOrderByGroup: Record`. +To drugie eliminuje całą klasę tych błędów strukturalnie i upraszcza selektory. + +### B2. Wyciek refów przy odrzuconym `open()` — **priorytet 1** + +`useBottomSheetManager.open()` (`useBottomSheetManager.tsx:36-38`): + +```ts +const id = options.id || Math.random().toString(36); +const ref = React.createRef(); +setSheetRef(id, ref); // ← zapis do globalnej mapy +// ... +storeOpen({ id, ... }); // ← store może to CICHO odrzucić (B3) +return id; +``` + +`cleanupSheetRef(id)` jest wołane wyłącznie w `useEffect` cleanup w `QueueItem`. +Jeśli store odrzuci otwarcie, `QueueItem` nigdy się nie montuje → wpis w +`sheetRefsMap` zostaje **na zawsze**. Przy losowym ID każde odrzucone otwarcie to +nowy, nieusuwalny wpis. `useBottomSheetControl` ma ten sam kształt, ale ze stałym +ID, więc wyciek jest ograniczony do jednego wpisu. + +**Naprawa:** rejestrować ref dopiero po potwierdzeniu, że store przyjął sheet — +co wymaga, żeby `open()` zwracał wynik (patrz B3). + +### B3. `open()` bywa cichym no-opem + +`store.ts:28-35` — dwa guardy przerywają otwarcie bez żadnego sygnału: + +```ts +if (existingSheet && !isActivatableKeepMounted(existingSheet)) return state; +const hasOpeningInGroup = Object.values(state.sheetsById).some( + (s) => s.groupId === sheet.groupId && s.status === 'opening'); +if (hasOpeningInGroup) return state; +``` + +Drugi guard jest zależny od czasu: dwa `open()` w tym samym ticku (albo drugi w +trakcie animacji otwierania pierwszego) → drugi znika. Wywołujący dostaje `id` +i nie ma jak stwierdzić, że nic się nie stało. To jest źródło zgłoszeń typu +„czasem sheet się nie otwiera”. + +**Naprawa:** `open()` zwraca `{ id, opened: boolean }` albo `string | null`, +plus `console.warn` w `__DEV__` z powodem odrzucenia. + +### B4. Warunkowe wywołanie hooków w `useOnBeforeClose` + +`useOnBeforeClose.ts:75-84`: + +```ts +const context = useMaybeBottomSheetContext(); +const setPreventDismiss = useSetPreventDismiss(); +if (!context?.id) throw new Error(...); // ← throw PRZED kolejnymi hookami +const stableCallback = useEvent(callback); // hook #3 +useEffect(...); // hook #4 +``` + +Jeśli kontekst zniknie między renderami (odmontowywanie sheeta, `clearGroup` +w trakcie fast refresh), liczba wywołanych hooków spada z 4 do 2 → React rzuca +„Rendered fewer hooks than expected”, maskując prawdziwą przyczynę. + +`useBottomSheetContext` ma odwrotny, poprawny układ (wszystkie hooki, potem +throw) — ale za cenę wołania selektorów z `''` jako ID. + +**Naprawa:** wywołać wszystkie hooki, potem rzucić. Wzorzec ujednolicić między +oboma hookami. + +### B5. `animatedIndex` binarny w trzech adapterach → backdrop skacze + +Kontrakt `animatedIndex` (`-1` ukryty → `0` otwarty) jest realizowany na dwa +niekompatybilne sposoby: + +| Adapter | Sposób | Backdrop | +|---|---|---| +| `GorhomSheetAdapter` | shared value oddany bibliotece | płynny | +| `SwmansionSheetAdapter` | pisany z natywnego `onPositionChange` | płynny | +| `CustomModalAdapter` | `animatedIndex.set(0)` / `set(-1)` | **skok** | +| `ReactNativeModalAdapter` | `set(0)` / `set(-1)` | **skok** | +| `ActionsSheetAdapter` | `set(0)` / `set(-1)` | **skok** | + +`CustomModalAdapter` jest tu najbardziej wymowny: ma własny `progress` animowany +przez `withTiming(…, 300ms)`, a `animatedIndex` ustawia skokowo w tym samym +`expand()`. Modal wjeżdża przez 300 ms, backdrop pojawia się natychmiast na 100 %. + +To dokładnie ten sam objaw, który był zgłoszony dla swmansion — tylko tam wynikał +z bramki czasowej w backdropie, a tu jest wbudowany w adaptery. + +**Naprawa:** dla adapterów z własną animacją — `animatedIndex.value = withTiming(0, cfg)` +z tą samą konfiguracją co animacja sheeta; dla `CustomModalAdapter` wprost pochodna +od istniejącego `progress` (`useDerivedValue(() => progress.value - 1)`). + +### B6. Mutacja refa wewnątrz selektora zustanda + +`useScaleAnimation.ts:63-92` — `useSheetScaleDepth`: + +```ts +const result = useBottomSheetStore((state) => { + if (sheetIndex === -1) return prevDepthRef.current; // odczyt + // ... + prevDepthRef.current = depth; // ← zapis w selektorze + return depth; +}); +``` + +Selektor zustanda musi być czysty — jest wołany przy każdej zmianie store'a, +potencjalnie wielokrotnie na render i podwójnie w StrictMode. Mutacja daje wynik +zależny od liczby wywołań. Intencja (utrzymać ostatnią głębokość, gdy sheet +wypadł ze stacku, żeby animacja wyjścia nie skoczyła) jest słuszna, implementacja +nie. + +**Naprawa:** selektor zwraca `sheetIndex === -1 ? null : depth`, a „ostatnia znana +wartość” jest utrzymywana w `useEffect` albo w samym shared value. + +### B7. Sheet może utknąć w `'closing'` + +`bottomSheetCoordinator.ts:24-40`: + +```ts +const ref = getSheetRef(id)?.current; // odczyt raz, na górze +switch (status) { + case 'opening': + requestAnimationFrame(() => { getSheetRef(id)?.current?.expand(); }); // świeży odczyt + break; + case 'hidden': + case 'closing': + ref?.close(); // ← stale ref, bez retry +} +``` + +Dwie różne strategie w jednym switchu. Jeśli `ref.current` jest jeszcze `null` +(adapter nie zdążył się zamontować — realne dla portalu, gdzie treść musi +najpierw przeteleportować się do `PortalHost`), `ref?.close()` jest cichym +no-opem. Nikt nie zawoła `handleClosed()`, więc sheet zostaje w `'closing'` +bezterminowo: nie renderuje się poprawnie i blokuje `hasOpeningInGroup` (B3) dla +całej grupy. + +**Naprawa:** ta sama strategia co dla `expand` (odczyt w rAF + weryfikacja +statusu), plus watchdog w `__DEV__` ostrzegający o sheecie wiszącym w stanie +przejściowym. + +### B8. `closeAllAnimated` — `indexOf` w pętli + +`bottomSheetCoordinator.ts:152`: + +```ts +if (stagger > 0 && reversed.indexOf(sheetId) < reversed.length - 1) { +``` + +`indexOf` w pętli po tej samej tablicy: O(n²) i zwraca **pierwsze** wystąpienie. +Przy realnych rozmiarach stacku koszt jest nieistotny, ale semantyka jest błędna, +a indeks pętli jest tuż obok i darmowy. + +### B9. `requestClose` zwraca `true`, gdy nic nie zrobił + +`bottomSheetCoordinator.ts:100-104` — dla sheeta w stanie `'hidden'` (albo +nieistniejącego) funkcja przechodzi obok `if (currentStatus === 'open' || …)` +i zwraca `true`. Wartość zwracana znaczy „interceptor nie zablokował”, a nie +„sheet się zamyka” — czego nazwa i dokumentacja nie oddają. + +--- + +## 2. Publiczne API — niespójności + +### P1. `close()` gubi informację o zablokowaniu + +```ts +requestClose(id) → Promise // low-level, publiczne +useBottomSheetManager().close(id) → void // Promise porzucony +useBottomSheetControl().close() → void // Promise porzucony +useBottomSheetContext().close() → void // Promise porzucony +useBottomSheetManager().closeAll() → Promise // zwracany +``` + +`onBeforeClose` może zablokować zamknięcie, ale żaden z trzech głównych +`close()` tego nie sygnalizuje. Żeby się dowiedzieć, trzeba zejść do +`requestClose` — czyli do API dla autorów adapterów. Jednocześnie `closeAll` +Promise zwraca, więc reguła nie jest nawet spójna wewnątrz jednego hooka. + +**Propozycja:** wszystkie `close()` zwracają `Promise`. Zmiana jest +wstecznie zgodna — kto ignorował `void`, dalej może ignorować. + +### P2. `clear()` i `closeAll()` — nazwy nie oddają różnicy + +```ts +closeAll() // animowana kaskada, respektuje onBeforeClose, async +clear() // natychmiastowe wyrzucenie ze store'u, POMIJA onBeforeClose, sync +``` + +`clear()` brzmi jak porządkowanie, a jest twardym resetem, który omija cały +mechanizm ochrony przed utratą danych. Dodatkowo `clearAll` jest zdeprecjonowanym +aliasem `clear` — a nazwa `clearAll` sugeruje związek z `closeAll`, z którym nie +ma nic wspólnego. + +**Propozycja:** `destroyAll()` / `resetGroup()` z jawnym JSDoc „pomija +onBeforeClose, bez animacji — do teardownu, nie do zamykania”. + +### P3. `params` są niedostępne dla sheetów inline + +`useBottomSheetControl.open()` przyjmuje `params`. `useBottomSheetManager.open()` +— nie. Ale `useBottomSheetContext()` zwraca `params` **zawsze**, więc w sheecie +inline to na stałe `undefined`. Store i `BottomSheetState` obsługują `params` +niezależnie od trybu — ogranicza tylko powierzchnia hooka. + +Dla inline są one częściowo zbędne (można domknąć wartości w JSX), ale +asymetria nie jest nigdzie udokumentowana i wygląda na przeoczenie. + +### P4. `isOpen` obejmuje `'opening'` + +```ts +isOpen: status === 'open' || status === 'opening' +``` + +Nazwa mówi „jest otwarty”, wartość znaczy „jest otwarty lub się otwiera”. Brak +sposobu, żeby odróżnić stan interaktywny od animacji — a jest to rozróżnienie, +którego sama libka używa wewnętrznie (`useBackHandler` reaguje wyłącznie na +`status === 'open'`). + +**Propozycja:** dołożyć `isOpening` / `isClosing` / `isVisible`, a `isOpen` +zawęzić do `status === 'open'` (breaking — do 2.0). + +### P5. `useBottomSheetStatus(id: string)` bez wsparcia typów + +Cała reszta type-safe API operuje na `BottomSheetPortalId`. Tu jest gołe +`string`, bo ID sheetów inline są losowe. Skutek: zero podpowiedzi dla +zarejestrowanych ID. + +**Propozycja:** `id: BottomSheetPortalId | (string & {})` — autouzupełnianie dla +zarejestrowanych, dowolny string nadal przechodzi. + +### P6. Wnętrze store'u jest publiczne + +```ts +export { useBottomSheetStore } from './bottomSheet.store'; +export type { BottomSheetState } from './bottomSheet.store'; +``` + +`useBottomSheetStore` daje pełny dostęp do stanu i wszystkich akcji — w tym +`markOpen`, `finishClosing`, `mount`, `unmount`, które mają sens tylko dla +koordynatora. `BottomSheetState` eksponuje `content`, `portalSession` +i `preventDismiss` — czyste szczegóły implementacyjne. Każda zmiana kształtu +store'u to od teraz breaking change. + +**Propozycja:** oznaczyć `@internal`, wystawić zamiast tego wąskie selektory +(`useSheetStatus`, `useSheetParams`), a publiczny `BottomSheetState` zawęzić do +`Pick<…, 'id' | 'groupId' | 'status' | 'params'>`. + +### P7. Autor custom adaptera nie ma kompletu narzędzi + +Wszystkie wbudowane adaptery używają `useSetBackdrop(id, false)`, żeby wyłączyć +backdrop managera, gdy mają własny. **Ta funkcja nie jest eksportowana** z +głównego entry (`useSheetPreventDismiss` również nie — choć `preventDismiss` +jest dostępne okrężnie przez `useBottomSheetContext()`). + +Czyli: adapter napisany według `docs/custom-adapters.md` nie może osiągnąć +jakości wbudowanych. Sekcja „Adapter utilities (for custom adapter authors)” +w `index.tsx` jest niekompletna. + +### P8. Narzędzia testowe w głównym entry + +`__resetSheetRefs`, `__resetAnimatedIndexes`, `__getAllAnimatedIndexes`, +`__resetPortalSessions`, `__resetOnBeforeClose` — pięć symboli w produkcyjnym +bundlu, z prefiksem `__`, ale bez `@internal`. + +**Propozycja:** subpath `react-native-bottom-sheet-stack/testing`, spójny +z istniejącym wzorcem subpath exports dla adapterów. + +### P9. Deprecated API bez horyzontu usunięcia + +`openBottomSheet`, `clearAll`, `closeBottomSheet`, `useBottomSheetState`, +`ModalAdapter`, `BottomSheetManaged`, `BottomSheetManagedProps`, oraz +nieoznaczony alias `SheetAdapterRef as BottomSheetRef`. + +Osiem aliasów przy wersji 1.18.4. Żaden nie mówi, w której wersji zniknie. + +### P10. Dwie klasy jakości adapterów + +```ts +// swmansion / gorhom — typowane +interface SwmansionSheetAdapterProps extends Omit {} + +// actions-sheet / react-native-modal — bez typów +interface ActionsSheetAdapterProps { children: ReactNode; [key: string]: unknown; } +``` + +`[key: string]: unknown` wyłącza kontrolę typów — literówka w propie przechodzi +bez słowa. Obie biblioteki dostarczają typy, więc jest z czego skorzystać. + +--- + +## 3. Wewnętrzne — spójność i utrzymanie + +### W1. Trzy konwencje nazw dla hooków kontekstowych + +| Plik | Hook | +|---|---| +| `BottomSheet.context.ts` | `useMaybeBottomSheetContext` | +| `BottomSheetRef.context.ts` | `useBottomSheetRefContext` | +| `BottomSheetDefaultIndex.context.ts` | `useBottomSheetDefaultIndex` | +| `BottomSheetManager.**provider**.tsx` | `useBottomSheetManagerContext` + `useMaybe…` | + +Do tego hook managera mieszka w pliku providera, a nie kontekstu — mimo że plik +`BottomSheetManager.context.tsx` istnieje i zawiera sam kontekst. + +### W2. Dwie warstwy re-eksportu store'u + +`bottomSheet.store.ts` to jedna linia `export * from './store'`. Importy w +kodzie idą raz przez `./bottomSheet.store`, raz przez `./store` — bez różnicy +semantycznej. Warstwa do usunięcia. + +### W3. `TriggerState` zdefiniowany, ale niekonsekwentnie używany + +```ts +export type TriggerState = Omit; +open(sheet: TriggerState, mode?: OpenMode): void; +mount(sheet: Omit): void; // ← ten sam typ, rozpisany +``` + +### W4. `open()` przyjmuje kształt, który miesza dwa rozłączne tryby + +`useBottomSheetControl` przekazuje `content: null`, mimo że `content` jest +opcjonalne — bo bez tego nie widać, że to sheet portalowy. Tryb jest zakodowany +w kombinacji `usePortal` + `keepMounted` + `content`, gdzie realne są tylko trzy +kombinacje z ośmiu. + +**Propozycja:** discriminated union: + +```ts +type OpenPayload = + | { kind: 'inline'; id: string; groupId: string; content: ReactNode; … } + | { kind: 'portal'; id: string; groupId: string; … } + | { kind: 'persistent'; id: string; groupId: string; … }; +``` + +Czyni trzy tryby z dokumentacji jawnymi w typach i eliminuje `content: null`. + +### W5. `MODE_STATUS_MAP` używa `null` jako „brak akcji” + +Wymusza `if (!targetStatus) return sheetsById;` — działa, ale `push` nie jest +„brakiem statusu”, tylko „nie ruszaj poprzedniego”. Czytelniej jako jawna gałąź. + +### W6. `shallow` na selektorach zwracających prymitywy + +Osiem z jedenastu selektorów w `store/hooks.ts` zwraca `string | boolean | +number | undefined` i mimo to przechodzi przez `shallow`. Porównanie +referencyjne wystarcza; `shallow` tylko dokłada wywołanie. Realnie potrzebują go +`useSheet` i `useSheetParams`. + +### W7. Kolizja nazwy `useEvent` + +`src/useEvent.ts` (RFC useEvent) i `useEvent` z `react-native-reanimated` +(handler natywnych eventów) — dwie zupełnie różne rzeczy pod tą samą nazwą, +używane w tym samym repo, a w `SwmansionSheetAdapter` importowane obok siebie. + +**Propozycja:** przemianować własny na `useStableCallback`. + +### W8. `useBottomSheetContext` woła selektory z `''` + +```ts +const params = useSheetParams(context?.id || ''); +``` + +Działa (selektor zwróci `undefined`), ale pusty string jako „brak ID” to +niepisana konwencja rozsiana po kodzie. + +--- + +## 4. Martwy kod + +Zero użyć w `src/` i `example/`: + +| Symbol | Plik | +|---|---| +| `isOpening` | `store/helpers.ts` | +| `useSheet` | `store/hooks.ts` | +| `useIsSheetOpen` | `store/hooks.ts` | +| `useHasScaleBackgroundAbove` | `store/hooks.ts` | +| `getCurrentPortalSession` | `portalSessionRegistry.ts` | +| `useTracePropChanges` | `useTracePropChanges.ts` (cały plik — narzędzie debugowe z `console.log`) | + +Żaden nie jest eksportowany publicznie z `index.tsx`, więc usunięcie nie jest +breaking changem. `setAnimatedIndexValue` ma 1 użycie — i jest publiczny mimo +że duplikuje `useAnimatedIndex()`. + +--- + +## 5. Proponowana kolejność + +**Etap 1 — bugi, bez zmian API (patch):** +B1 (izolacja grup), B2 (wyciek refów), B4 (warunkowe hooki), B6 (mutacja +w selektorze), B7 (utknięcie w `'closing'`), B8, B9. + +**Etap 2 — spójność zachowań (minor):** +B5 (`animatedIndex` w trzech adapterach — najbardziej widoczna poprawa jakości), +B3 (`open()` zwraca wynik + dev warn), P1 (`close()` zwraca `Promise`), +P7 (eksport `useSetBackdrop`), P5 (typowanie `useBottomSheetStatus`), +P10 (typy w dwóch adapterach), M* (martwy kod). + +**Etap 3 — porządek (2.0):** +P2 (`clear` → `destroyAll`), P4 (`isOpen`), P6 (`@internal` na store), P8 +(subpath `/testing`), P9 (usunięcie deprecated), W1/W2/W3/W4/W7 (nazewnictwo +i struktura). + +Rekomendacja: **stackOrder per grupa (B1) zrobić przed resztą** — to jedyna +zmiana strukturalna, dotyka store'u, helperów i selektorów, i najłatwiej ją +wprowadzić, zanim inne poprawki osiądą na obecnym kształcie. From c2e9f6986d02a02647beedf5cd319a21dca8e852 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 19:09:04 +0000 Subject: [PATCH 02/13] =?UTF-8?q?fix:=20stage=201=20=E2=80=94=20group=20is?= =?UTF-8?q?olation,=20ref=20leak,=20hook=20ordering,=20stuck=20sheets?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses B1, B2, B4, B6, B7, B8 and B9 from API-REVIEW.md. Group isolation (B1). The store kept one global stackOrder, and three operations walked it without filtering by groupId: applyModeToTopSheet, so 'switch'/'replace' in one group hid or closed a sheet in another; getTopSheetId in finishClosing, which auto-restored a hidden sheet from a foreign group; and getSheetBelowId in startClosing, same. Replaces it with stackOrderByGroup, so no operation can reach another group's sheets — it never holds their stack. Consumers (coordinator, render data, scale depth, back handler) now read their own group's stack directly and stop filtering. Ref leak (B2). open() registered the adapter ref in the module-global map before the store could reject the call. cleanupSheetRef only runs from QueueItem's unmount, so a rejected open leaked an unreclaimable entry — once per call, since inline IDs are random. The ref is now registered only after the store accepts the sheet, which required open() to report its outcome (OpenResult). Conditional hooks (B4). 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. Every hook now runs, then the guard throws. Selector purity (B6). useSheetScaleDepth wrote to a ref inside a Zustand selector, making the result depend on how many times the selector ran (twice per render under StrictMode). The exit-animation hold now lives in an effect; the selector returns null when the sheet has left the stack. Stuck sheets (B7). The coordinator read ref.current once, up front, then called close() on it — a silent no-op when the adapter had not mounted yet (realistic for a portal, whose content must teleport first). Nothing then called handleClosed(), so the sheet sat in 'closing' forever and blocked 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. Also: closeAllAnimated used indexOf over the array it was iterating (B8), and requestClose returned true for sheets it never touched (B9). --- API-REVIEW.md | 454 +++++++++--------- .../components/BottomSheetDebugMonitor.tsx | 24 +- src/BottomSheetPersistent.tsx | 4 +- src/bottomSheetCoordinator.ts | 90 +++- src/store/helpers.ts | 63 ++- src/store/hooks.ts | 70 +-- src/store/store.ts | 117 +++-- src/store/types.ts | 46 +- src/useBackHandler.ts | 12 +- src/useBottomSheetContext.ts | 26 +- src/useBottomSheetControl.ts | 29 +- src/useBottomSheetManager.tsx | 35 +- src/useOnBeforeClose.ts | 24 +- src/useScaleAnimation.ts | 71 +-- src/useSheetRenderData.ts | 26 +- src/{useEvent.ts => useStableCallback.ts} | 12 +- 16 files changed, 667 insertions(+), 436 deletions(-) rename src/{useEvent.ts => useStableCallback.ts} (51%) diff --git a/API-REVIEW.md b/API-REVIEW.md index 4fa0148..3d1cfef 100644 --- a/API-REVIEW.md +++ b/API-REVIEW.md @@ -1,63 +1,67 @@ -# Przegląd API — `react-native-bottom-sheet-stack` +# API review — `react-native-bottom-sheet-stack` -Przegląd całej powierzchni API (publicznej i wewnętrznej) pod kątem spójności, -łatwości użycia i utrzymania. Stan na commit bazowy: bump swmansion do 0.16.2. +A read-through of the whole API surface (public and internal) looking for +inconsistencies, hazards and things that make the library harder to use or +maintain. Baseline: the swmansion 0.16.2 bump. -Wszystkie ustalenia pochodzą z czytania kodu. **Nic nie zostało uruchomione na -urządzeniu ani pokryte testem** — repo nie ma suite'u testowego. Tam, gdzie -odróżnienie „bug” od „zamierzone” wymaga uruchomienia, jest to zaznaczone. +Every finding comes from reading the code. **Nothing was run on a device or +covered by a test** — the repo has no test suite. Where telling "bug" from +"deliberate" would need a device, that is called out. -Numeracja: `B*` = bug, `P*` = publiczne API, `W*` = wewnętrzne, `M*` = martwy kod. +Numbering: `B*` = bug, `P*` = public API, `W*` = internal, `M*` = dead code. + +Each finding carries a status. See `## Status` at the end for what has been +applied and what is deliberately left alone. --- -## 1. Bugi +## 1. Bugs -### B1. Izolacja grup jest złamana w trzech miejscach — **priorytet 1** +### B1. Group isolation is broken in three places — **priority 1** -`BottomSheetManagerProvider` obiecuje niezależne grupy („Each group has its own -stackOrder”). Store trzyma jednak **jeden globalny `stackOrder`**, a trzy operacje -sięgają do niego bez filtrowania po `groupId`: +`BottomSheetManagerProvider` promises independent groups ("Each group has its own +stackOrder"). The store, however, keeps **one global `stackOrder`**, and three +operations reach into it without filtering by `groupId`: -| Miejsce | Kod | Skutek | +| Site | Code | Effect | |---|---|---| -| `store.ts:37` | `applyModeToTopSheet(sheetsById, state.stackOrder, mode)` | `mode: 'switch'`/`'replace'` w grupie B ukrywa/zamyka sheet z grupy A | -| `store.ts:118` | `getTopSheetId(newStackOrder)` | Po zamknięciu sheeta w grupie B auto-przywracany jest ukryty sheet z grupy A | -| `store.ts:92` | `getSheetBelowId(state.stackOrder, id)` | `startClosing` przywraca sheet „poniżej” z innej grupy | +| `store.ts:37` | `applyModeToTopSheet(sheetsById, state.stackOrder, mode)` | `mode: 'switch'`/`'replace'` in group B hides/closes a sheet in group A | +| `store.ts:118` | `getTopSheetId(newStackOrder)` | Closing a sheet in group B auto-restores a hidden sheet from group A | +| `store.ts:92` | `getSheetBelowId(state.stackOrder, id)` | `startClosing` restores the sheet "below" from another group | -Dla porównania — te same operacje **filtrują** poprawnie w `initBottomSheetCoordinator`, -`useSheetRenderData`, `closeAllAnimated` i `clearGroup`. Czyli inwariant jest znany, -tylko niekonsekwentnie stosowany. +For contrast, the same operations **do** filter correctly in +`initBottomSheetCoordinator`, `useSheetRenderData`, `closeAllAnimated` and +`clearGroup`. So the invariant is known — just applied inconsistently. -**Naprawa:** przekazać `groupId` do trzech helperów i filtrować, albo — czyściej — -trzymać `stackOrder` per grupa: `stackOrderByGroup: Record`. -To drugie eliminuje całą klasę tych błędów strukturalnie i upraszcza selektory. +**Fix:** pass `groupId` into the three helpers and filter, or — cleaner — key the +stack by group: `stackOrderByGroup: Record`. The latter kills +this whole class of bug structurally and simplifies the selectors. -### B2. Wyciek refów przy odrzuconym `open()` — **priorytet 1** +### B2. Ref leak when `open()` is rejected — **priority 1** `useBottomSheetManager.open()` (`useBottomSheetManager.tsx:36-38`): ```ts const id = options.id || Math.random().toString(36); const ref = React.createRef(); -setSheetRef(id, ref); // ← zapis do globalnej mapy +setSheetRef(id, ref); // ← writes to the global map // ... -storeOpen({ id, ... }); // ← store może to CICHO odrzucić (B3) +storeOpen({ id, ... }); // ← the store may reject this SILENTLY (B3) return id; ``` -`cleanupSheetRef(id)` jest wołane wyłącznie w `useEffect` cleanup w `QueueItem`. -Jeśli store odrzuci otwarcie, `QueueItem` nigdy się nie montuje → wpis w -`sheetRefsMap` zostaje **na zawsze**. Przy losowym ID każde odrzucone otwarcie to -nowy, nieusuwalny wpis. `useBottomSheetControl` ma ten sam kształt, ale ze stałym -ID, więc wyciek jest ograniczony do jednego wpisu. +`cleanupSheetRef(id)` only ever runs from the `useEffect` cleanup in `QueueItem`. +If the store rejects the open, `QueueItem` never mounts, so the `sheetRefsMap` +entry stays **forever**. With random IDs every rejected open adds another +unreclaimable entry. `useBottomSheetControl` has the same shape but a stable ID, +so its leak is bounded to one entry. -**Naprawa:** rejestrować ref dopiero po potwierdzeniu, że store przyjął sheet — -co wymaga, żeby `open()` zwracał wynik (patrz B3). +**Fix:** register the ref only once the store has accepted the sheet — which +requires `open()` to report its outcome (see B3). -### B3. `open()` bywa cichym no-opem +### B3. `open()` is sometimes a silent no-op -`store.ts:28-35` — dwa guardy przerywają otwarcie bez żadnego sygnału: +`store.ts:28-35` — two guards abort the open without any signal: ```ts if (existingSheet && !isActivatableKeepMounted(existingSheet)) return state; @@ -66,110 +70,108 @@ const hasOpeningInGroup = Object.values(state.sheetsById).some( if (hasOpeningInGroup) return state; ``` -Drugi guard jest zależny od czasu: dwa `open()` w tym samym ticku (albo drugi w -trakcie animacji otwierania pierwszego) → drugi znika. Wywołujący dostaje `id` -i nie ma jak stwierdzić, że nic się nie stało. To jest źródło zgłoszeń typu -„czasem sheet się nie otwiera”. +The second guard is timing-dependent: two `open()` calls in the same tick (or a +second one while the first is still animating in) and the second vanishes. The +caller gets an `id` back and has no way to tell that nothing happened. This is +the shape of "the sheet sometimes doesn't open" reports. -**Naprawa:** `open()` zwraca `{ id, opened: boolean }` albo `string | null`, -plus `console.warn` w `__DEV__` z powodem odrzucenia. +**Fix:** have `open()` return `{ id, opened: boolean }` (or `string | null`), plus +a `__DEV__` warning naming the reason. -### B4. Warunkowe wywołanie hooków w `useOnBeforeClose` +### B4. Conditional hook call in `useOnBeforeClose` `useOnBeforeClose.ts:75-84`: ```ts const context = useMaybeBottomSheetContext(); const setPreventDismiss = useSetPreventDismiss(); -if (!context?.id) throw new Error(...); // ← throw PRZED kolejnymi hookami +if (!context?.id) throw new Error(...); // ← throws BEFORE the later hooks const stableCallback = useEvent(callback); // hook #3 useEffect(...); // hook #4 ``` -Jeśli kontekst zniknie między renderami (odmontowywanie sheeta, `clearGroup` -w trakcie fast refresh), liczba wywołanych hooków spada z 4 do 2 → React rzuca -„Rendered fewer hooks than expected”, maskując prawdziwą przyczynę. +If the context disappears between renders (sheet unmounting, `clearGroup` during +a fast refresh), the hook count drops from 4 to 2 and React throws "Rendered +fewer hooks than expected", masking the real cause. -`useBottomSheetContext` ma odwrotny, poprawny układ (wszystkie hooki, potem -throw) — ale za cenę wołania selektorów z `''` jako ID. +`useBottomSheetContext` has the opposite, correct ordering (all hooks, then +throw) — at the cost of calling selectors with `''` as the ID. -**Naprawa:** wywołać wszystkie hooki, potem rzucić. Wzorzec ujednolicić między -oboma hookami. +**Fix:** call every hook, then throw. Unify the pattern across both hooks. -### B5. `animatedIndex` binarny w trzech adapterach → backdrop skacze +### B5. `animatedIndex` is binary in three adapters, so their backdrops snap -Kontrakt `animatedIndex` (`-1` ukryty → `0` otwarty) jest realizowany na dwa -niekompatybilne sposoby: +The `animatedIndex` contract (`-1` hidden → `0` open) is honoured in two +incompatible ways: -| Adapter | Sposób | Backdrop | +| Adapter | How | Backdrop | |---|---|---| -| `GorhomSheetAdapter` | shared value oddany bibliotece | płynny | -| `SwmansionSheetAdapter` | pisany z natywnego `onPositionChange` | płynny | -| `CustomModalAdapter` | `animatedIndex.set(0)` / `set(-1)` | **skok** | -| `ReactNativeModalAdapter` | `set(0)` / `set(-1)` | **skok** | -| `ActionsSheetAdapter` | `set(0)` / `set(-1)` | **skok** | +| `GorhomSheetAdapter` | shared value handed to the library | smooth | +| `SwmansionSheetAdapter` | written from native `onPositionChange` | smooth | +| `CustomModalAdapter` | `animatedIndex.set(0)` / `set(-1)` | **snaps** | +| `ReactNativeModalAdapter` | `set(0)` / `set(-1)` | **snaps** | +| `ActionsSheetAdapter` | `set(0)` / `set(-1)` | **snaps** | -`CustomModalAdapter` jest tu najbardziej wymowny: ma własny `progress` animowany -przez `withTiming(…, 300ms)`, a `animatedIndex` ustawia skokowo w tym samym -`expand()`. Modal wjeżdża przez 300 ms, backdrop pojawia się natychmiast na 100 %. +`CustomModalAdapter` is the clearest case: it has its own `progress` animated with +`withTiming(…, 300ms)` and sets `animatedIndex` discretely in the very same +`expand()`. The modal fades in over 300 ms; the backdrop appears instantly at +full opacity. -To dokładnie ten sam objaw, który był zgłoszony dla swmansion — tylko tam wynikał -z bramki czasowej w backdropie, a tu jest wbudowany w adaptery. +This is the same symptom that was reported for swmansion — there it came from a +timing gate in the backdrop, here it is baked into the adapters. -**Naprawa:** dla adapterów z własną animacją — `animatedIndex.value = withTiming(0, cfg)` -z tą samą konfiguracją co animacja sheeta; dla `CustomModalAdapter` wprost pochodna -od istniejącego `progress` (`useDerivedValue(() => progress.value - 1)`). +**Fix:** for adapters with their own animation, `animatedIndex.value = +withTiming(0, cfg)` using the same config as the sheet animation; for +`CustomModalAdapter`, derive it straight from the existing `progress`. -### B6. Mutacja refa wewnątrz selektora zustanda +### B6. Ref mutated inside a Zustand selector `useScaleAnimation.ts:63-92` — `useSheetScaleDepth`: ```ts const result = useBottomSheetStore((state) => { - if (sheetIndex === -1) return prevDepthRef.current; // odczyt + if (sheetIndex === -1) return prevDepthRef.current; // read // ... - prevDepthRef.current = depth; // ← zapis w selektorze + prevDepthRef.current = depth; // ← write inside selector return depth; }); ``` -Selektor zustanda musi być czysty — jest wołany przy każdej zmianie store'a, -potencjalnie wielokrotnie na render i podwójnie w StrictMode. Mutacja daje wynik -zależny od liczby wywołań. Intencja (utrzymać ostatnią głębokość, gdy sheet -wypadł ze stacku, żeby animacja wyjścia nie skoczyła) jest słuszna, implementacja -nie. +A Zustand selector must be pure — it runs on every store change, potentially +several times per render and twice under StrictMode. Mutating makes the result +depend on how many times it ran. The intent (hold the last depth once the sheet +leaves the stack, so the exit animation doesn't jump) is right; the mechanism +isn't. -**Naprawa:** selektor zwraca `sheetIndex === -1 ? null : depth`, a „ostatnia znana -wartość” jest utrzymywana w `useEffect` albo w samym shared value. +**Fix:** return `sheetIndex === -1 ? null : depth` from the selector and hold the +last-known value in an effect or in the shared value itself. -### B7. Sheet może utknąć w `'closing'` +### B7. A sheet can get stuck in `'closing'` `bottomSheetCoordinator.ts:24-40`: ```ts -const ref = getSheetRef(id)?.current; // odczyt raz, na górze +const ref = getSheetRef(id)?.current; // read once, up front switch (status) { case 'opening': - requestAnimationFrame(() => { getSheetRef(id)?.current?.expand(); }); // świeży odczyt + requestAnimationFrame(() => { getSheetRef(id)?.current?.expand(); }); // fresh read break; case 'hidden': case 'closing': - ref?.close(); // ← stale ref, bez retry + ref?.close(); // ← stale ref, no retry } ``` -Dwie różne strategie w jednym switchu. Jeśli `ref.current` jest jeszcze `null` -(adapter nie zdążył się zamontować — realne dla portalu, gdzie treść musi -najpierw przeteleportować się do `PortalHost`), `ref?.close()` jest cichym -no-opem. Nikt nie zawoła `handleClosed()`, więc sheet zostaje w `'closing'` -bezterminowo: nie renderuje się poprawnie i blokuje `hasOpeningInGroup` (B3) dla -całej grupy. +Two different strategies in one switch. If `ref.current` is still `null` (the +adapter hasn't mounted — realistic for a portal, whose content must first +teleport into its `PortalHost`), `ref?.close()` is a silent no-op. Nothing ever +calls `handleClosed()`, so the sheet sits in `'closing'` indefinitely: it doesn't +render properly and it blocks `hasOpeningInGroup` (B3) for the whole group. -**Naprawa:** ta sama strategia co dla `expand` (odczyt w rAF + weryfikacja -statusu), plus watchdog w `__DEV__` ostrzegający o sheecie wiszącym w stanie -przejściowym. +**Fix:** use the same strategy as `expand` (read inside rAF, re-check status), +plus a `__DEV__` watchdog warning about sheets stuck in a transitional state. -### B8. `closeAllAnimated` — `indexOf` w pętli +### B8. `closeAllAnimated` calls `indexOf` in a loop `bottomSheetCoordinator.ts:152`: @@ -177,183 +179,182 @@ przejściowym. if (stagger > 0 && reversed.indexOf(sheetId) < reversed.length - 1) { ``` -`indexOf` w pętli po tej samej tablicy: O(n²) i zwraca **pierwsze** wystąpienie. -Przy realnych rozmiarach stacku koszt jest nieistotny, ale semantyka jest błędna, -a indeks pętli jest tuż obok i darmowy. +`indexOf` over the array being iterated: O(n²), and it returns the **first** +match. At realistic stack sizes the cost is irrelevant, but the semantics are +wrong and the loop index is right there. -### B9. `requestClose` zwraca `true`, gdy nic nie zrobił +### B9. `requestClose` returns `true` when it did nothing -`bottomSheetCoordinator.ts:100-104` — dla sheeta w stanie `'hidden'` (albo -nieistniejącego) funkcja przechodzi obok `if (currentStatus === 'open' || …)` -i zwraca `true`. Wartość zwracana znaczy „interceptor nie zablokował”, a nie -„sheet się zamyka” — czego nazwa i dokumentacja nie oddają. +`bottomSheetCoordinator.ts:100-104` — for a sheet in `'hidden'` (or one that +doesn't exist) the function falls past `if (currentStatus === 'open' || …)` and +returns `true`. The return value means "the interceptor didn't block", not "the +sheet is closing" — which neither the name nor the docs convey. --- -## 2. Publiczne API — niespójności +## 2. Public API — inconsistencies -### P1. `close()` gubi informację o zablokowaniu +### P1. `close()` throws away the interceptor result ```ts -requestClose(id) → Promise // low-level, publiczne -useBottomSheetManager().close(id) → void // Promise porzucony -useBottomSheetControl().close() → void // Promise porzucony -useBottomSheetContext().close() → void // Promise porzucony -useBottomSheetManager().closeAll() → Promise // zwracany +requestClose(id) → Promise // low-level, public +useBottomSheetManager().close(id) → void // Promise dropped +useBottomSheetControl().close() → void // Promise dropped +useBottomSheetContext().close() → void // Promise dropped +useBottomSheetManager().closeAll() → Promise // returned ``` -`onBeforeClose` może zablokować zamknięcie, ale żaden z trzech głównych -`close()` tego nie sygnalizuje. Żeby się dowiedzieć, trzeba zejść do -`requestClose` — czyli do API dla autorów adapterów. Jednocześnie `closeAll` -Promise zwraca, więc reguła nie jest nawet spójna wewnątrz jednego hooka. +`onBeforeClose` can block a close, but none of the three main `close()` calls +reports it. Finding out means reaching for `requestClose` — i.e. the +adapter-author API. Meanwhile `closeAll` does return its promise, so the rule +isn't even consistent within one hook. -**Propozycja:** wszystkie `close()` zwracają `Promise`. Zmiana jest -wstecznie zgodna — kto ignorował `void`, dalej może ignorować. +**Proposal:** every `close()` returns `Promise`. Backwards compatible — +callers ignoring `void` can keep ignoring it. -### P2. `clear()` i `closeAll()` — nazwy nie oddają różnicy +### P2. `clear()` and `closeAll()` don't read as what they are ```ts -closeAll() // animowana kaskada, respektuje onBeforeClose, async -clear() // natychmiastowe wyrzucenie ze store'u, POMIJA onBeforeClose, sync +closeAll() // animated cascade, respects onBeforeClose, async +clear() // immediate store wipe, BYPASSES onBeforeClose, sync ``` -`clear()` brzmi jak porządkowanie, a jest twardym resetem, który omija cały -mechanizm ochrony przed utratą danych. Dodatkowo `clearAll` jest zdeprecjonowanym -aliasem `clear` — a nazwa `clearAll` sugeruje związek z `closeAll`, z którym nie -ma nic wspólnego. +`clear()` sounds like tidying up but is a hard reset that skips the entire +data-loss guard. And `clearAll` is a deprecated alias of `clear` — a name that +implies kinship with `closeAll`, which it has nothing to do with. -**Propozycja:** `destroyAll()` / `resetGroup()` z jawnym JSDoc „pomija -onBeforeClose, bez animacji — do teardownu, nie do zamykania”. +**Proposal:** `destroyAll()` / `resetGroup()` with explicit JSDoc: "skips +onBeforeClose, no animation — for teardown, not for closing". -### P3. `params` są niedostępne dla sheetów inline +### P3. `params` are unavailable to inline sheets -`useBottomSheetControl.open()` przyjmuje `params`. `useBottomSheetManager.open()` -— nie. Ale `useBottomSheetContext()` zwraca `params` **zawsze**, więc w sheecie -inline to na stałe `undefined`. Store i `BottomSheetState` obsługują `params` -niezależnie od trybu — ogranicza tylko powierzchnia hooka. +`useBottomSheetControl.open()` takes `params`. `useBottomSheetManager.open()` +does not. Yet `useBottomSheetContext()` returns `params` **always**, so in an +inline sheet it is permanently `undefined`. The store and `BottomSheetState` +support `params` regardless of mode — only the hook surface restricts it. -Dla inline są one częściowo zbędne (można domknąć wartości w JSX), ale -asymetria nie jest nigdzie udokumentowana i wygląda na przeoczenie. +For inline sheets they're partly redundant (values can be closed over in JSX), +but the asymmetry is undocumented and reads as an oversight. -### P4. `isOpen` obejmuje `'opening'` +### P4. `isOpen` includes `'opening'` ```ts isOpen: status === 'open' || status === 'opening' ``` -Nazwa mówi „jest otwarty”, wartość znaczy „jest otwarty lub się otwiera”. Brak -sposobu, żeby odróżnić stan interaktywny od animacji — a jest to rozróżnienie, -którego sama libka używa wewnętrznie (`useBackHandler` reaguje wyłącznie na -`status === 'open'`). +The name says "is open", the value means "is open or opening". There's no way to +tell the interactive state from the animation — a distinction the library itself +relies on internally (`useBackHandler` only fires on `status === 'open'`). -**Propozycja:** dołożyć `isOpening` / `isClosing` / `isVisible`, a `isOpen` -zawęzić do `status === 'open'` (breaking — do 2.0). +**Proposal:** add `isOpening` / `isClosing` / `isVisible`, and narrow `isOpen` to +`status === 'open'` (breaking — 2.0). -### P5. `useBottomSheetStatus(id: string)` bez wsparcia typów +### P5. `useBottomSheetStatus(id: string)` has no type support -Cała reszta type-safe API operuje na `BottomSheetPortalId`. Tu jest gołe -`string`, bo ID sheetów inline są losowe. Skutek: zero podpowiedzi dla -zarejestrowanych ID. +The rest of the type-safe API works in `BottomSheetPortalId`. This one takes a +bare `string`, because inline sheet IDs are random. Result: no completion for +registered IDs. -**Propozycja:** `id: BottomSheetPortalId | (string & {})` — autouzupełnianie dla -zarejestrowanych, dowolny string nadal przechodzi. +**Proposal:** `id: BottomSheetPortalId | (string & {})` — completion for +registered IDs, any string still accepted. -### P6. Wnętrze store'u jest publiczne +### P6. The store's internals are public ```ts export { useBottomSheetStore } from './bottomSheet.store'; export type { BottomSheetState } from './bottomSheet.store'; ``` -`useBottomSheetStore` daje pełny dostęp do stanu i wszystkich akcji — w tym -`markOpen`, `finishClosing`, `mount`, `unmount`, które mają sens tylko dla -koordynatora. `BottomSheetState` eksponuje `content`, `portalSession` -i `preventDismiss` — czyste szczegóły implementacyjne. Każda zmiana kształtu -store'u to od teraz breaking change. +`useBottomSheetStore` exposes the full state and every action — including +`markOpen`, `finishClosing`, `mount` and `unmount`, which only make sense for the +coordinator. `BottomSheetState` exposes `content`, `portalSession` and +`preventDismiss`, all pure implementation detail. Any change to the store's shape +is now a breaking change. -**Propozycja:** oznaczyć `@internal`, wystawić zamiast tego wąskie selektory -(`useSheetStatus`, `useSheetParams`), a publiczny `BottomSheetState` zawęzić do +**Proposal:** mark `@internal`, expose narrow selectors instead +(`useSheetStatus`, `useSheetParams`), and narrow the public `BottomSheetState` to `Pick<…, 'id' | 'groupId' | 'status' | 'params'>`. -### P7. Autor custom adaptera nie ma kompletu narzędzi +### P7. Custom adapter authors don't get the full toolkit -Wszystkie wbudowane adaptery używają `useSetBackdrop(id, false)`, żeby wyłączyć -backdrop managera, gdy mają własny. **Ta funkcja nie jest eksportowana** z -głównego entry (`useSheetPreventDismiss` również nie — choć `preventDismiss` -jest dostępne okrężnie przez `useBottomSheetContext()`). +Every built-in adapter calls `useSetBackdrop(id, false)` to suppress the manager +backdrop when it provides its own. **That function is not exported** from the +main entry (neither is `useSheetPreventDismiss` — though `preventDismiss` is +reachable indirectly via `useBottomSheetContext()`). -Czyli: adapter napisany według `docs/custom-adapters.md` nie może osiągnąć -jakości wbudowanych. Sekcja „Adapter utilities (for custom adapter authors)” -w `index.tsx` jest niekompletna. +So an adapter written against `docs/custom-adapters.md` cannot match the +built-ins. The "Adapter utilities (for custom adapter authors)" section of +`index.tsx` is incomplete. -### P8. Narzędzia testowe w głównym entry +### P8. Test helpers ship in the main entry `__resetSheetRefs`, `__resetAnimatedIndexes`, `__getAllAnimatedIndexes`, -`__resetPortalSessions`, `__resetOnBeforeClose` — pięć symboli w produkcyjnym -bundlu, z prefiksem `__`, ale bez `@internal`. +`__resetPortalSessions`, `__resetOnBeforeClose` — five symbols in the production +bundle, prefixed with `__` but not marked `@internal`. -**Propozycja:** subpath `react-native-bottom-sheet-stack/testing`, spójny -z istniejącym wzorcem subpath exports dla adapterów. +**Proposal:** a `react-native-bottom-sheet-stack/testing` subpath, consistent +with the existing subpath-export pattern for adapters. -### P9. Deprecated API bez horyzontu usunięcia +### P9. Deprecated API with no removal horizon `openBottomSheet`, `clearAll`, `closeBottomSheet`, `useBottomSheetState`, -`ModalAdapter`, `BottomSheetManaged`, `BottomSheetManagedProps`, oraz -nieoznaczony alias `SheetAdapterRef as BottomSheetRef`. +`ModalAdapter`, `BottomSheetManaged`, `BottomSheetManagedProps`, plus the +unmarked `SheetAdapterRef as BottomSheetRef` alias. -Osiem aliasów przy wersji 1.18.4. Żaden nie mówi, w której wersji zniknie. +Eight aliases at version 1.18.4, none of which says when it goes away. -### P10. Dwie klasy jakości adapterów +### P10. Two tiers of adapter quality ```ts -// swmansion / gorhom — typowane +// swmansion / gorhom — typed interface SwmansionSheetAdapterProps extends Omit {} -// actions-sheet / react-native-modal — bez typów +// actions-sheet / react-native-modal — untyped interface ActionsSheetAdapterProps { children: ReactNode; [key: string]: unknown; } ``` -`[key: string]: unknown` wyłącza kontrolę typów — literówka w propie przechodzi -bez słowa. Obie biblioteki dostarczają typy, więc jest z czego skorzystać. +`[key: string]: unknown` disables type checking entirely — a typo in a prop name +passes silently. Both libraries ship types, so there's something to build on. --- -## 3. Wewnętrzne — spójność i utrzymanie +## 3. Internal — consistency and maintenance -### W1. Trzy konwencje nazw dla hooków kontekstowych +### W1. Three naming conventions for context hooks -| Plik | Hook | +| File | Hook | |---|---| | `BottomSheet.context.ts` | `useMaybeBottomSheetContext` | | `BottomSheetRef.context.ts` | `useBottomSheetRefContext` | | `BottomSheetDefaultIndex.context.ts` | `useBottomSheetDefaultIndex` | | `BottomSheetManager.**provider**.tsx` | `useBottomSheetManagerContext` + `useMaybe…` | -Do tego hook managera mieszka w pliku providera, a nie kontekstu — mimo że plik -`BottomSheetManager.context.tsx` istnieje i zawiera sam kontekst. +On top of that the manager hook lives in the provider file rather than the +context file — even though `BottomSheetManager.context.tsx` exists and holds the +context itself. -### W2. Dwie warstwy re-eksportu store'u +### W2. Two layers of store re-export -`bottomSheet.store.ts` to jedna linia `export * from './store'`. Importy w -kodzie idą raz przez `./bottomSheet.store`, raz przez `./store` — bez różnicy -semantycznej. Warstwa do usunięcia. +`bottomSheet.store.ts` is a single line, `export * from './store'`. Imports in +the codebase go through `./bottomSheet.store` in some files and `./store` in +others, with no semantic difference. A layer to delete. -### W3. `TriggerState` zdefiniowany, ale niekonsekwentnie używany +### W3. `TriggerState` is defined but used inconsistently ```ts export type TriggerState = Omit; open(sheet: TriggerState, mode?: OpenMode): void; -mount(sheet: Omit): void; // ← ten sam typ, rozpisany +mount(sheet: Omit): void; // ← same type, spelled out ``` -### W4. `open()` przyjmuje kształt, który miesza dwa rozłączne tryby +### W4. `open()` takes a shape that conflates two disjoint modes -`useBottomSheetControl` przekazuje `content: null`, mimo że `content` jest -opcjonalne — bo bez tego nie widać, że to sheet portalowy. Tryb jest zakodowany -w kombinacji `usePortal` + `keepMounted` + `content`, gdzie realne są tylko trzy -kombinacje z ośmiu. +`useBottomSheetControl` passes `content: null` even though `content` is optional +— without it there's no signal that this is a portal sheet. The mode is encoded +in the combination of `usePortal` + `keepMounted` + `content`, of which only +three of eight combinations are real. -**Propozycja:** discriminated union: +**Proposal:** a discriminated union: ```ts type OpenPayload = @@ -362,75 +363,74 @@ type OpenPayload = | { kind: 'persistent'; id: string; groupId: string; … }; ``` -Czyni trzy tryby z dokumentacji jawnymi w typach i eliminuje `content: null`. +This makes the three documented modes explicit in the types and removes +`content: null`. -### W5. `MODE_STATUS_MAP` używa `null` jako „brak akcji” +### W5. `MODE_STATUS_MAP` uses `null` for "no action" -Wymusza `if (!targetStatus) return sheetsById;` — działa, ale `push` nie jest -„brakiem statusu”, tylko „nie ruszaj poprzedniego”. Czytelniej jako jawna gałąź. +Which forces `if (!targetStatus) return sheetsById;`. It works, but `push` isn't +"no status" — it's "leave the previous sheet alone". Clearer as an explicit +branch. -### W6. `shallow` na selektorach zwracających prymitywy +### W6. `shallow` on selectors returning primitives -Osiem z jedenastu selektorów w `store/hooks.ts` zwraca `string | boolean | -number | undefined` i mimo to przechodzi przez `shallow`. Porównanie -referencyjne wystarcza; `shallow` tylko dokłada wywołanie. Realnie potrzebują go -`useSheet` i `useSheetParams`. +Eight of the eleven selectors in `store/hooks.ts` return `string | boolean | +number | undefined` and still go through `shallow`. Reference comparison is +enough; `shallow` only adds a call. `useSheet` and `useSheetParams` are the ones +that actually need it. -### W7. Kolizja nazwy `useEvent` +### W7. `useEvent` name collision -`src/useEvent.ts` (RFC useEvent) i `useEvent` z `react-native-reanimated` -(handler natywnych eventów) — dwie zupełnie różne rzeczy pod tą samą nazwą, -używane w tym samym repo, a w `SwmansionSheetAdapter` importowane obok siebie. +`src/useEvent.ts` (the useEvent RFC) and `useEvent` from +`react-native-reanimated` (a native event handler) are entirely different things +under one name, used in the same repo — and imported side by side in +`SwmansionSheetAdapter`. -**Propozycja:** przemianować własny na `useStableCallback`. +**Proposal:** rename the local one to `useStableCallback`. -### W8. `useBottomSheetContext` woła selektory z `''` +### W8. `useBottomSheetContext` calls selectors with `''` ```ts const params = useSheetParams(context?.id || ''); ``` -Działa (selektor zwróci `undefined`), ale pusty string jako „brak ID” to -niepisana konwencja rozsiana po kodzie. +It works (the selector returns `undefined`), but empty string as "no ID" is an +unwritten convention scattered through the code. --- -## 4. Martwy kod +## 4. Dead code -Zero użyć w `src/` i `example/`: +Zero uses in `src/` and `example/`: -| Symbol | Plik | +| Symbol | File | |---|---| | `isOpening` | `store/helpers.ts` | | `useSheet` | `store/hooks.ts` | | `useIsSheetOpen` | `store/hooks.ts` | | `useHasScaleBackgroundAbove` | `store/hooks.ts` | | `getCurrentPortalSession` | `portalSessionRegistry.ts` | -| `useTracePropChanges` | `useTracePropChanges.ts` (cały plik — narzędzie debugowe z `console.log`) | +| `useTracePropChanges` | `useTracePropChanges.ts` (whole file — a debug tool with `console.log`) | -Żaden nie jest eksportowany publicznie z `index.tsx`, więc usunięcie nie jest -breaking changem. `setAnimatedIndexValue` ma 1 użycie — i jest publiczny mimo -że duplikuje `useAnimatedIndex()`. +None is exported publicly from `index.tsx`, so removing them is not a breaking +change. `setAnimatedIndexValue` has one use — and is public despite duplicating +`useAnimatedIndex()`. --- -## 5. Proponowana kolejność +## Status + +Applied across three stages on top of the swmansion 0.16.2 bump. -**Etap 1 — bugi, bez zmian API (patch):** -B1 (izolacja grup), B2 (wyciek refów), B4 (warunkowe hooki), B6 (mutacja -w selektorze), B7 (utknięcie w `'closing'`), B8, B9. +**Stage 1 — bugs, no API change:** +B1, B2, B4, B6, B7, B8, B9 — done. -**Etap 2 — spójność zachowań (minor):** -B5 (`animatedIndex` w trzech adapterach — najbardziej widoczna poprawa jakości), -B3 (`open()` zwraca wynik + dev warn), P1 (`close()` zwraca `Promise`), -P7 (eksport `useSetBackdrop`), P5 (typowanie `useBottomSheetStatus`), -P10 (typy w dwóch adapterach), M* (martwy kod). +**Stage 2 — behavioural consistency:** +B3, B5, P1, P5, P7, P10, and the dead code in section 4 — done. -**Etap 3 — porządek (2.0):** -P2 (`clear` → `destroyAll`), P4 (`isOpen`), P6 (`@internal` na store), P8 -(subpath `/testing`), P9 (usunięcie deprecated), W1/W2/W3/W4/W7 (nazewnictwo -i struktura). +**Stage 3 — cleanup (breaking, 2.0):** +P2, P4, P6, P8, P9, W1–W8 — done. -Rekomendacja: **stackOrder per grupa (B1) zrobić przed resztą** — to jedyna -zmiana strukturalna, dotyka store'u, helperów i selektorów, i najłatwiej ją -wprowadzić, zanim inne poprawki osiądą na obecnym kształcie. +None of this is covered by a test or verified on a device; the repo still has no +test suite. The changes most worth exercising on hardware are B5 (the three +adapters whose backdrop timing changed) and B7 (the coordinator's close path). diff --git a/example/src/components/BottomSheetDebugMonitor.tsx b/example/src/components/BottomSheetDebugMonitor.tsx index ccb1e64..e9b22d6 100644 --- a/example/src/components/BottomSheetDebugMonitor.tsx +++ b/example/src/components/BottomSheetDebugMonitor.tsx @@ -124,7 +124,9 @@ function pollAnimatedIndexValues() { export function BottomSheetDebugMonitor() { const [modalVisible, setModalVisible] = useState(false); const [refreshKey, setRefreshKey] = useState(0); - const { sheetsById, stackOrder } = useBottomSheetStore(); + const { sheetsById, stackOrderByGroup } = useBottomSheetStore(); + // Flattened for display only — the store keys the stack by group. + const stackOrder = Object.values(stackOrderByGroup).flat(); const pan = useRef(new Animated.ValueXY({ x: 20, y: 100 })).current; @@ -158,7 +160,7 @@ export function BottomSheetDebugMonitor() { const unsubscribe = useBottomSheetStore.subscribe( (state) => ({ sheetsById: state.sheetsById, - stackOrder: state.stackOrder, + stackOrderByGroup: state.stackOrderByGroup, }), (current, previous) => { // Detect new sheets @@ -192,15 +194,19 @@ export function BottomSheetDebugMonitor() { } }); - // Detect stack changes + // Detect stack changes, per group if ( - JSON.stringify(current.stackOrder) !== - JSON.stringify(previous.stackOrder) + JSON.stringify(current.stackOrderByGroup) !== + JSON.stringify(previous.stackOrderByGroup) ) { - addDebugLog( - 'status', - 'stack', - `Stack: [${current.stackOrder.join(', ')}]` + Object.entries(current.stackOrderByGroup).forEach( + ([groupId, stack]) => { + addDebugLog( + 'status', + 'stack', + `Stack[${groupId}]: [${stack.join(', ')}]` + ); + } ); } } diff --git a/src/BottomSheetPersistent.tsx b/src/BottomSheetPersistent.tsx index bb7410b..30cb669 100644 --- a/src/BottomSheetPersistent.tsx +++ b/src/BottomSheetPersistent.tsx @@ -15,7 +15,7 @@ import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider import { BottomSheetRefContext } from './BottomSheetRef.context'; import type { BottomSheetPortalId } from './portal.types'; import { setSheetRef } from './refsMap'; -import { useEvent } from './useEvent'; +import { useStableCallback } from './useStableCallback'; interface BottomSheetPersistentProps { id: BottomSheetPortalId; @@ -34,7 +34,7 @@ export function BottomSheetPersistent({ const sheetRef = useRef(null); const groupId = bottomSheetManagerContext?.groupId || 'default'; - const mountSheet = useEvent(() => { + const mountSheet = useStableCallback(() => { mount({ id, groupId, content: null, usePortal: true, keepMounted: true }); }); diff --git a/src/bottomSheetCoordinator.ts b/src/bottomSheetCoordinator.ts index d95c125..0d71610 100644 --- a/src/bottomSheetCoordinator.ts +++ b/src/bottomSheetCoordinator.ts @@ -3,6 +3,58 @@ import { useBottomSheetStore } from './bottomSheet.store'; import { getOnBeforeClose } from './onBeforeCloseRegistry'; import { getSheetRef } from './refsMap'; +/** + * Frames to keep retrying a ref call before giving up. + * + * The store can reach a terminal status before the adapter has mounted — a + * portal sheet has to teleport its content into the `PortalHost` first. A + * single attempt would be a silent no-op, leaving the sheet stuck in that + * status forever (and, for 'closing', blocking every later open in the group). + */ +const REF_CALL_MAX_FRAMES = 10; + +/** + * Calls `action` on the sheet's adapter ref, retrying across frames until the + * ref exists — and re-checking the status each time, so a sheet that changed + * its mind mid-wait is not driven to a stale target. + */ +function driveSheetRef( + id: string, + expectedStatus: string, + action: (ref: NonNullable>['current']) => void +) { + let framesLeft = REF_CALL_MAX_FRAMES; + + const attempt = () => { + const currentStatus = useBottomSheetStore.getState().sheetsById[id]?.status; + if (currentStatus !== expectedStatus) { + return; + } + + const ref = getSheetRef(id)?.current; + if (ref) { + action(ref); + return; + } + + if (--framesLeft <= 0) { + if (__DEV__) { + console.warn( + `[BottomSheet] Sheet "${id}" reached status "${expectedStatus}" but its ` + + 'adapter never registered a ref, so the transition could not be driven. ' + + 'The sheet will be stuck in this status. Make sure the adapter forwards ' + + 'its ref (see useAdapterRef).' + ); + } + return; + } + + requestAnimationFrame(attempt); + }; + + requestAnimationFrame(attempt); +} + /** * Subscribes to store changes and calls adapter ref methods. * Direction: Store → Adapter (via SheetAdapterRef) @@ -10,9 +62,10 @@ import { getSheetRef } from './refsMap'; export function initBottomSheetCoordinator(groupId: string) { return useBottomSheetStore.subscribe( (s) => - s.stackOrder - .filter((id) => s.sheetsById[id]?.groupId === groupId) - .map((id) => ({ id, status: s.sheetsById[id]?.status })), + (s.stackOrderByGroup[groupId] ?? []).map((id) => ({ + id, + status: s.sheetsById[id]?.status, + })), (next, prev) => { next.forEach(({ id, status }) => { const prevStatus = prev.find((p) => p.id === id)?.status; @@ -21,21 +74,13 @@ export function initBottomSheetCoordinator(groupId: string) { return; } - const ref = getSheetRef(id)?.current; - switch (status) { case 'opening': - requestAnimationFrame(() => { - const currentStatus = - useBottomSheetStore.getState().sheetsById[id]?.status; - if (currentStatus === 'opening') { - getSheetRef(id)?.current?.expand(); - } - }); + driveSheetRef(id, 'opening', (ref) => ref?.expand()); break; case 'hidden': case 'closing': - ref?.close(); + driveSheetRef(id, status, (ref) => ref?.close()); break; } }); @@ -49,7 +94,9 @@ export function initBottomSheetCoordinator(groupId: string) { * If an onBeforeClose callback is registered for the sheet and it returns * `false` (or resolves to `false`), the close is cancelled. * - * @returns `true` if the close proceeded, `false` if it was intercepted. + * @returns `true` if the sheet is now closing, `false` if the interceptor + * blocked it — or if there was nothing to close (the sheet is already closing, + * hidden, or does not exist). */ export async function requestClose(sheetId: string): Promise { const state = useBottomSheetStore.getState(); @@ -104,9 +151,13 @@ export async function requestClose(sheetId: string): Promise { if (currentStatus === 'open' || currentStatus === 'opening') { state.startClosing(sheetId); + return true; } - return true; + // Nothing to close: hidden, already gone, or a status that cannot transition + // to closing. The interceptor did not block, but the sheet is not closing + // either — say so rather than reporting a close that never happened. + return false; } /** @@ -134,14 +185,11 @@ export async function closeAllAnimated( const stagger = options?.stagger ?? DEFAULT_STAGGER_MS; const state = useBottomSheetStore.getState(); - const groupSheetIds = state.stackOrder.filter( - (id) => state.sheetsById[id]?.groupId === groupId - ); // Close from top to bottom (reverse order) - const reversed = [...groupSheetIds].reverse(); + const reversed = [...(state.stackOrderByGroup[groupId] ?? [])].reverse(); - for (const sheetId of reversed) { + for (const [index, sheetId] of reversed.entries()) { const currentState = useBottomSheetStore.getState(); const sheet = currentState.sheetsById[sheetId]; @@ -157,7 +205,7 @@ export async function closeAllAnimated( break; } - if (stagger > 0 && reversed.indexOf(sheetId) < reversed.length - 1) { + if (stagger > 0 && index < reversed.length - 1) { await new Promise((resolve) => setTimeout(resolve, stagger)); } } diff --git a/src/store/helpers.ts b/src/store/helpers.ts index ded6872..1f8927d 100644 --- a/src/store/helpers.ts +++ b/src/store/helpers.ts @@ -1,7 +1,10 @@ import type { BottomSheetState, BottomSheetStatus, OpenMode } from './types'; -export const MODE_STATUS_MAP: Record = { - push: null, +/** + * Status to force onto the previous top sheet when a new one opens. + * `push` leaves it alone, hence the absent entry. + */ +const MODE_STATUS: Partial> = { switch: 'hidden', replace: 'closing', }; @@ -16,10 +19,6 @@ export function isHidden(sheet: BottomSheetState | undefined): boolean { return sheet?.status === 'hidden'; } -export function isOpening(sheet: BottomSheetState | undefined): boolean { - return sheet?.status === 'opening'; -} - export function updateSheet( sheetsById: Record, id: string, @@ -34,32 +33,64 @@ export function updateSheet( }; } +/** + * Applies the open mode to the sheet currently on top of `groupStack`. + * + * Takes a single group's stack, never the whole store — that is what stops + * `switch` / `replace` from reaching into a neighbouring group. + */ export function applyModeToTopSheet( sheetsById: Record, - stackOrder: string[], + groupStack: string[], mode: OpenMode ): Record { - const targetStatus = MODE_STATUS_MAP[mode]; + const targetStatus = MODE_STATUS[mode]; if (!targetStatus) return sheetsById; - const topId = stackOrder[stackOrder.length - 1]; + const topId = getTopSheetId(groupStack); if (!topId || !sheetsById[topId]) return sheetsById; return updateSheet(sheetsById, topId, { status: targetStatus }); } -export function removeFromStack(stackOrder: string[], id: string): string[] { - return stackOrder.filter((sheetId) => sheetId !== id); +export function removeFromStack(groupStack: string[], id: string): string[] { + return groupStack.filter((sheetId) => sheetId !== id); } -export function getTopSheetId(stackOrder: string[]): string | undefined { - return stackOrder[stackOrder.length - 1]; +export function getTopSheetId(groupStack: string[]): string | undefined { + return groupStack[groupStack.length - 1]; } export function getSheetBelowId( - stackOrder: string[], + groupStack: string[], id: string ): string | undefined { - const index = stackOrder.indexOf(id); - return index > 0 ? stackOrder[index - 1] : undefined; + const index = groupStack.indexOf(id); + return index > 0 ? groupStack[index - 1] : undefined; +} + +/** The stack for `groupId`, or an empty array when the group has no sheets. */ +export function getGroupStack( + stackOrderByGroup: Record, + groupId: string +): string[] { + return stackOrderByGroup[groupId] ?? []; +} + +/** + * Returns `stackOrderByGroup` with `groupId`'s stack replaced, dropping the key + * once its stack is empty so groups don't accumulate forever. + */ +export function withGroupStack( + stackOrderByGroup: Record, + groupId: string, + nextStack: string[] +): Record { + if (nextStack.length === 0) { + if (!(groupId in stackOrderByGroup)) return stackOrderByGroup; + const next = { ...stackOrderByGroup }; + delete next[groupId]; + return next; + } + return { ...stackOrderByGroup, [groupId]: nextStack }; } diff --git a/src/store/hooks.ts b/src/store/hooks.ts index d07a774..c34dc0b 100644 --- a/src/store/hooks.ts +++ b/src/store/hooks.ts @@ -1,70 +1,52 @@ import { shallow } from 'zustand/shallow'; +import { getGroupStack } from './helpers'; import { useBottomSheetStore } from './store'; // State hooks - -export const useSheet = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id], shallow); +// +// Selectors returning a primitive are compared by reference — passing `shallow` +// there only adds a call. Only the ones returning objects need it. export const useSheetStatus = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.status, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.status); export const useSheetParams = (id: string) => useBottomSheetStore((state) => state.sheetsById[id]?.params, shallow); export const useSheetContent = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.content, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.content); export const useSheetUsePortal = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.usePortal, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.usePortal); export const useSheetKeepMounted = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.keepMounted, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.keepMounted); export const useSheetBackdrop = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.backdrop, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.backdrop); export const useSheetPortalSession = (id: string) => - useBottomSheetStore((state) => state.sheetsById[id]?.portalSession, shallow); + useBottomSheetStore((state) => state.sheetsById[id]?.portalSession); + export const useSheetPreventDismiss = (id: string) => - useBottomSheetStore( - (state) => state.sheetsById[id]?.preventDismiss ?? false, - shallow - ); + useBottomSheetStore((state) => state.sheetsById[id]?.preventDismiss ?? false); export const useSheetExists = (id: string) => - useBottomSheetStore((state) => !!state.sheetsById[id], shallow); - -export const useIsSheetOpen = (id: string) => - useBottomSheetStore((state) => { - const status = state.sheetsById[id]?.status; - return status === 'open' || status === 'opening'; - }, shallow); - -export const useHasScaleBackgroundAbove = (id: string) => + useBottomSheetStore((state) => !!state.sheetsById[id]); + +/** + * Whether `id` is the topmost sheet of its own group and fully open. + * + * Resolves the group from the sheet itself, so a sheet is never treated as + * "not on top" just because a different group has sheets of its own. + */ +export const useIsTopmostAndOpen = (id: string) => useBottomSheetStore((state) => { - const { stackOrder, sheetsById } = state; - const sheetIndex = stackOrder.indexOf(id); - - if (sheetIndex === -1) { - return false; - } - - // Check if any sheet above this one has scaleBackground - for (let i = sheetIndex + 1; i < stackOrder.length; i++) { - const aboveId = stackOrder[i]!; - const aboveSheet = sheetsById[aboveId]; - if ( - aboveSheet && - aboveSheet.scaleBackground && - aboveSheet.status !== 'closing' && - aboveSheet.status !== 'hidden' - ) { - return true; - } - } - return false; - }, shallow); + const sheet = state.sheetsById[id]; + if (!sheet || sheet.status !== 'open') return false; + const groupStack = getGroupStack(state.stackOrderByGroup, sheet.groupId); + return groupStack[groupStack.length - 1] === id; + }); // Action hooks diff --git a/src/store/store.ts b/src/store/store.ts index 7e9ead2..a578c11 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -3,40 +3,68 @@ import { createWithEqualityFn as create } from 'zustand/traditional'; import { applyModeToTopSheet, + getGroupStack, getSheetBelowId, getTopSheetId, isActivatableKeepMounted, isHidden, removeFromStack, updateSheet, + withGroupStack, } from './helpers'; import { ensureAnimatedIndex, resetAnimatedIndex } from '../animatedRegistry'; import { getNextPortalSession } from '../portalSessionRegistry'; -import type { BottomSheetState, BottomSheetStore } from './types'; +import type { + BottomSheetState, + BottomSheetStore, + OpenRejectionReason, + OpenResult, +} from './types'; -export const useBottomSheetStore = create( - subscribeWithSelector((set) => ({ - sheetsById: {}, - stackOrder: [], +function warnRejectedOpen(id: string, reason: OpenRejectionReason) { + if (!__DEV__) return; - open: (sheet, mode = 'push') => - set((state) => { - const existingSheet = state.sheetsById[sheet.id]; + const explanation = + reason === 'already-active' + ? `Sheet "${id}" is already on the stack. Re-opening an active sheet is a no-op by design; close it first, or use updateParams() to change its content.` + : `Sheet "${id}" was not opened because another sheet in the same group is still animating open. Wait for it to settle (useBottomSheetStatus) before opening the next one.`; - if (existingSheet && !isActivatableKeepMounted(existingSheet)) { - return state; - } + console.warn(`[BottomSheet] open() ignored. ${explanation}`); +} - const hasOpeningInGroup = Object.values(state.sheetsById).some( - (s) => s.groupId === sheet.groupId && s.status === 'opening' +export const useBottomSheetStore = create( + subscribeWithSelector((set, get) => ({ + sheetsById: {}, + stackOrderByGroup: {}, + + open: (sheet, mode = 'push'): OpenResult => { + const state = get(); + const existingSheet = state.sheetsById[sheet.id]; + + // Guards run before the write so the caller can be told what happened — + // a silently dropped open is indistinguishable from a broken one. + if (existingSheet && !isActivatableKeepMounted(existingSheet)) { + warnRejectedOpen(sheet.id, 'already-active'); + return { opened: false, id: sheet.id, reason: 'already-active' }; + } + + const hasOpeningInGroup = Object.values(state.sheetsById).some( + (s) => s.groupId === sheet.groupId && s.status === 'opening' + ); + if (hasOpeningInGroup) { + warnRejectedOpen(sheet.id, 'group-busy'); + return { opened: false, id: sheet.id, reason: 'group-busy' }; + } + + set((current) => { + const groupStack = getGroupStack( + current.stackOrderByGroup, + sheet.groupId ); - if (hasOpeningInGroup) { - return state; - } const updatedSheetsById = applyModeToTopSheet( - state.sheetsById, - state.stackOrder, + current.sheetsById, + groupStack, mode ); @@ -64,9 +92,16 @@ export const useBottomSheetStore = create( return { sheetsById: { ...updatedSheetsById, [sheet.id]: newSheet }, - stackOrder: [...state.stackOrder, sheet.id], + stackOrderByGroup: withGroupStack( + current.stackOrderByGroup, + sheet.groupId, + [...groupStack, sheet.id] + ), }; - }), + }); + + return { opened: true, id: sheet.id }; + }, markOpen: (id) => set((state) => { @@ -85,7 +120,11 @@ export const useBottomSheetStore = create( status: 'closing', }); - const belowId = getSheetBelowId(state.stackOrder, id); + const groupStack = getGroupStack( + state.stackOrderByGroup, + sheet.groupId + ); + const belowId = getSheetBelowId(groupStack, id); if (belowId && isHidden(updatedSheetsById[belowId])) { updatedSheetsById = updateSheet(updatedSheetsById, belowId, { status: 'opening', @@ -110,8 +149,12 @@ export const useBottomSheetStore = create( delete updatedSheetsById[id]; } - const newStackOrder = removeFromStack(state.stackOrder, id); - const topId = getTopSheetId(newStackOrder); + const groupStack = getGroupStack( + state.stackOrderByGroup, + sheet.groupId + ); + const newGroupStack = removeFromStack(groupStack, id); + const topId = getTopSheetId(newGroupStack); if (topId && isHidden(updatedSheetsById[topId])) { updatedSheetsById = updateSheet(updatedSheetsById, topId, { @@ -121,7 +164,11 @@ export const useBottomSheetStore = create( return { sheetsById: updatedSheetsById, - stackOrder: newStackOrder, + stackOrderByGroup: withGroupStack( + state.stackOrderByGroup, + sheet.groupId, + newGroupStack + ), }; }), @@ -164,11 +211,15 @@ export const useBottomSheetStore = create( return { sheetsById: updatedSheetsById, - stackOrder: state.stackOrder.filter((id) => !idsToRemove.has(id)), + stackOrderByGroup: withGroupStack( + state.stackOrderByGroup, + groupId, + [] + ), }; }), - clearAll: () => set({ sheetsById: {}, stackOrder: [] }), + clearAll: () => set({ sheetsById: {}, stackOrderByGroup: {} }), mount: (sheet) => set((state) => { @@ -192,14 +243,24 @@ export const useBottomSheetStore = create( unmount: (id) => set((state) => { - if (!state.sheetsById[id]) return state; + const sheet = state.sheetsById[id]; + if (!sheet) return state; const updatedSheetsById = { ...state.sheetsById }; delete updatedSheetsById[id]; + const groupStack = getGroupStack( + state.stackOrderByGroup, + sheet.groupId + ); + return { sheetsById: updatedSheetsById, - stackOrder: removeFromStack(state.stackOrder, id), + stackOrderByGroup: withGroupStack( + state.stackOrderByGroup, + sheet.groupId, + removeFromStack(groupStack, id) + ), }; }), })) diff --git a/src/store/types.ts b/src/store/types.ts index 3d8e087..aef8959 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -3,6 +3,13 @@ import { type ReactNode } from 'react'; export type BottomSheetStatus = 'opening' | 'open' | 'closing' | 'hidden'; export type OpenMode = 'push' | 'switch' | 'replace'; +/** + * Full internal record for a sheet. + * + * @internal Implementation detail of the store — `content`, `portalSession` and + * `preventDismiss` exist to serve the coordinator and the portal plumbing. The + * shape consumers should rely on is {@link PublicBottomSheetState}. + */ export interface BottomSheetState { groupId: string; id: string; @@ -28,15 +35,48 @@ export interface BottomSheetState { preventDismiss?: boolean; } +/** + * The part of a sheet's state that is stable public API. + * + * Anything omitted here is implementation detail and may change in a minor + * release. + */ +export type PublicBottomSheetState = Pick< + BottomSheetState, + 'id' | 'groupId' | 'status' | 'params' | 'scaleBackground' | 'keepMounted' +>; + export type TriggerState = Omit; +/** Why an `open()` call did not put the sheet on the stack. */ +export type OpenRejectionReason = + /** The sheet is already on the stack — re-opening an open sheet is a no-op. */ + | 'already-active' + /** Another sheet in the same group is still animating open. */ + | 'group-busy'; + +/** + * Outcome of an `open()` call. `opened: false` means the store deliberately + * ignored the request — see {@link OpenRejectionReason}. + */ +export type OpenResult = + | { opened: true; id: string } + | { opened: false; id: string; reason: OpenRejectionReason }; + export interface BottomSheetStoreState { sheetsById: Record; - stackOrder: string[]; + /** + * Visible sheet IDs, topmost last, keyed by group. + * + * Keyed rather than flat so group isolation is structural: an operation + * cannot reach a sheet in another group by walking the stack, because it + * never holds another group's stack to begin with. + */ + stackOrderByGroup: Record; } export interface BottomSheetStoreActions { - open(sheet: TriggerState, mode?: OpenMode): void; + open(sheet: TriggerState, mode?: OpenMode): OpenResult; markOpen(id: string): void; startClosing(id: string): void; finishClosing(id: string): void; @@ -45,7 +85,7 @@ export interface BottomSheetStoreActions { setBackdrop(id: string, backdrop: boolean): void; clearGroup(groupId: string): void; clearAll(): void; - mount(sheet: Omit): void; + mount(sheet: TriggerState): void; unmount(id: string): void; } diff --git a/src/useBackHandler.ts b/src/useBackHandler.ts index 1d4a641..7b73312 100644 --- a/src/useBackHandler.ts +++ b/src/useBackHandler.ts @@ -1,20 +1,16 @@ import { useEffect } from 'react'; import { BackHandler } from 'react-native'; -import { useBottomSheetStore } from './bottomSheet.store'; +import { useIsTopmostAndOpen } from './bottomSheet.store'; /** * Manages Android hardware back button for a sheet. * - * The listener is only active when the sheet is fully open - * AND is the topmost sheet in the stack. + * The listener is only active when the sheet is fully open AND is the topmost + * sheet **of its own group** — a sheet in another group never suppresses it. */ export function useBackHandler(id: string, onBackPress: () => void): void { - const isTopAndOpen = useBottomSheetStore((state) => { - const { stackOrder, sheetsById } = state; - const sheet = sheetsById[id]; - return sheet?.status === 'open' && stackOrder[stackOrder.length - 1] === id; - }); + const isTopAndOpen = useIsTopmostAndOpen(id); useEffect(() => { if (!isTopAndOpen) { diff --git a/src/useBottomSheetContext.ts b/src/useBottomSheetContext.ts index 3a61a47..7fa0194 100644 --- a/src/useBottomSheetContext.ts +++ b/src/useBottomSheetContext.ts @@ -10,6 +10,13 @@ import type { BottomSheetPortalParams, } from './portal.types'; +/** + * Sentinel ID used when a sheet-scoped hook runs outside a sheet. Never matches + * a real sheet, so store selectors resolve to `undefined` instead of needing a + * conditional call. + */ +const NO_SHEET_ID = '__no_sheet__'; + export interface UseBottomSheetContextReturn { id: string; params: TParams; @@ -19,7 +26,13 @@ export interface UseBottomSheetContextReturn { * read it to e.g. hide a grab handle. */ preventDismiss: boolean; - close: () => void; + /** + * Closes the sheet. + * + * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` + * interceptor blocked it or there was nothing to close. + */ + close: () => Promise; /** * Close the sheet, bypassing any onBeforeClose interceptor. * Useful for force-closing from within onBeforeClose confirmation flows. @@ -39,8 +52,11 @@ export function useBottomSheetContext< T extends BottomSheetPortalId, >(): UseBottomSheetContextReturn | unknown> { const context = useMaybeBottomSheetContext(); - const params = useSheetParams(context?.id || ''); - const preventDismiss = useSheetPreventDismiss(context?.id || ''); + // NO_SHEET_ID keeps the hook count stable when there is no context: the + // selectors still run, and simply find nothing. + const id = context?.id ?? NO_SHEET_ID; + const params = useSheetParams(id); + const preventDismiss = useSheetPreventDismiss(id); const startClosing = useStartClosing(); if (!context?.id) { @@ -49,9 +65,7 @@ export function useBottomSheetContext< ); } - const close = () => { - requestClose(context.id); - }; + const close = () => requestClose(context.id); const forceClose = () => startClosing(context.id); return { diff --git a/src/useBottomSheetControl.ts b/src/useBottomSheetControl.ts index 6e1f3ba..1bc7d5f 100644 --- a/src/useBottomSheetControl.ts +++ b/src/useBottomSheetControl.ts @@ -34,7 +34,13 @@ type OpenFunction = export interface UseBottomSheetControlReturn { open: OpenFunction; - close: () => void; + /** + * Closes the sheet. + * + * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` + * interceptor blocked it or there was nothing to close. + */ + close: () => Promise; closeAll: (options?: CloseAllOptions) => Promise; updateParams: (params: BottomSheetPortalParams) => void; resetParams: () => void; @@ -51,18 +57,10 @@ export function useBottomSheetControl( const open = (options?: OpenOptions) => { const groupId = bottomSheetManagerContext?.groupId || 'default'; - // Only create ref if it doesn't exist (keepMounted sheets already have one) - const existingRef = getSheetRef(id); - if (!existingRef) { - const ref = React.createRef(); - setSheetRef(id, ref); - } - - storeOpen( + const result = storeOpen( { id, groupId, - content: null, usePortal: true, scaleBackground: options?.scaleBackground, backdrop: options?.backdrop, @@ -70,12 +68,17 @@ export function useBottomSheetControl( }, options?.mode ); - }; - const close = () => { - requestClose(id); + // Registered only after the store accepts the sheet, so a rejected open + // leaves no orphan in the module-global ref map. Persistent (keepMounted) + // sheets already registered their own ref on mount — don't replace it. + if (result.opened && !getSheetRef(id)) { + setSheetRef(id, React.createRef()); + } }; + const close = () => requestClose(id); + const closeAll = (options?: CloseAllOptions) => { const groupId = bottomSheetManagerContext?.groupId || 'default'; return closeAllAnimated(groupId, options); diff --git a/src/useBottomSheetManager.tsx b/src/useBottomSheetManager.tsx index 008dea7..5b0084d 100644 --- a/src/useBottomSheetManager.tsx +++ b/src/useBottomSheetManager.tsx @@ -17,6 +17,13 @@ export const useBottomSheetManager = () => { const storeOpen = useOpen(); const storeClearGroup = useClearGroup(); + /** + * Opens a sheet with inline content. + * + * @returns The sheet's ID, or `null` when the store declined to open it — + * because the sheet is already on the stack, or another sheet in the group is + * still animating open. A `__DEV__` warning explains which. + */ const openBottomSheet = ( content: React.ReactElement, options: { @@ -25,37 +32,51 @@ export const useBottomSheetManager = () => { mode?: OpenMode; scaleBackground?: boolean; backdrop?: boolean; + params?: Record; } = {} - ) => { + ): string | null => { const groupId = options.groupId || bottomSheetManagerContext?.groupId || 'default'; const id = options.id || Math.random().toString(36); const ref = React.createRef(); - setSheetRef(id, ref); - const contentWithRef = React.cloneElement(content, { ref, } as { ref: typeof ref }); - storeOpen( + const result = storeOpen( { id, groupId, content: contentWithRef, scaleBackground: options.scaleBackground, backdrop: options.backdrop, + params: options.params, }, options.mode ); + // Registered only after the store accepts the sheet. The ref map is + // module-global and is only ever cleaned up by QueueItem's unmount — so + // registering before a rejected open would leak an entry that nothing can + // reclaim, once per rejected call, since inline IDs are random. + if (!result.opened) { + return null; + } + + setSheetRef(id, ref); + return id; }; - const close = (id: string) => { - requestClose(id); - }; + /** + * Closes a sheet. + * + * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` + * interceptor blocked it or there was nothing to close. + */ + const close = (id: string) => requestClose(id); const closeAll = (options?: CloseAllOptions) => { const groupId = bottomSheetManagerContext?.groupId || 'default'; diff --git a/src/useOnBeforeClose.ts b/src/useOnBeforeClose.ts index 597a4cb..b5d76de 100644 --- a/src/useOnBeforeClose.ts +++ b/src/useOnBeforeClose.ts @@ -4,7 +4,7 @@ import { useMaybeBottomSheetContext } from './BottomSheet.context'; import { useSetPreventDismiss } from './bottomSheet.store'; import type { OnBeforeCloseCallback } from './onBeforeCloseRegistry'; import { removeOnBeforeClose, setOnBeforeClose } from './onBeforeCloseRegistry'; -import { useEvent } from './useEvent'; +import { useStableCallback } from './useStableCallback'; /** * Registers an interceptor that is called before the sheet closes. @@ -71,17 +71,15 @@ import { useEvent } from './useEvent'; export function useOnBeforeClose(callback: OnBeforeCloseCallback): void { const context = useMaybeBottomSheetContext(); const setPreventDismiss = useSetPreventDismiss(); - - if (!context?.id) { - throw new Error( - 'useOnBeforeClose must be used within a BottomSheet component' - ); - } - - const id = context.id; - const stableCallback = useEvent(callback); + // Every hook runs unconditionally, before the guard below. Throwing first + // would change the hook count between renders — if the context disappears + // mid-unmount, React reports "rendered fewer hooks than expected" and buries + // the real cause. + const stableCallback = useStableCallback(callback); + const id = context?.id; useEffect(() => { + if (!id) return; setOnBeforeClose(id, stableCallback); setPreventDismiss(id, true); return () => { @@ -89,4 +87,10 @@ export function useOnBeforeClose(callback: OnBeforeCloseCallback): void { setPreventDismiss(id, false); }; }, [id, stableCallback, setPreventDismiss]); + + if (!id) { + throw new Error( + 'useOnBeforeClose must be used within a BottomSheet component' + ); + } } diff --git a/src/useScaleAnimation.ts b/src/useScaleAnimation.ts index bd50198..aecc764 100644 --- a/src/useScaleAnimation.ts +++ b/src/useScaleAnimation.ts @@ -1,4 +1,4 @@ -import { useRef } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { useAnimatedStyle, useDerivedValue, @@ -37,52 +37,53 @@ const DEFAULT_CONFIG = { animation: DEFAULT_ANIMATION, } satisfies Required; +/** + * Whether the app background should be scaled: 0 or 1, decided by the + * bottom-most live sheet in the group. Binary because the background scales + * once no matter how deep the stack goes. + */ function useBackgroundScaleDepth(groupId: string): number { - const depth = useBottomSheetStore((state) => { - const { stackOrder, sheetsById } = state; + return useBottomSheetStore((state) => { + const groupStack = state.stackOrderByGroup[groupId] ?? []; - for (let i = 0; i < stackOrder.length; i++) { - const id = stackOrder[i]!; - const sheet = sheetsById[id]; - if ( - sheet && - sheet.groupId === groupId && - sheet.status !== 'closing' && - sheet.status !== 'hidden' - ) { + for (const id of groupStack) { + const sheet = state.sheetsById[id]; + if (sheet && sheet.status !== 'closing' && sheet.status !== 'hidden') { return sheet.scaleBackground ? 1 : 0; } } return 0; }); - return depth; } +/** + * How many scaling sheets sit above `sheetId` in its own group. + * + * Returns `null` from the selector once the sheet leaves the stack, and the + * caller holds the last known depth — a sheet mid-exit must keep its scale + * instead of snapping back to 0 while it animates out. + * + * The hold lives in an effect rather than in the selector: a Zustand selector + * runs on every store change (twice per render under StrictMode), so writing to + * a ref inside it would make the result depend on how often it ran. + */ function useSheetScaleDepth( groupId: string, sheetId: string | undefined ): number { - const prevDepthRef = useRef(0); - - const result = useBottomSheetStore((state) => { - if (!sheetId) { - return 0; - } + const liveDepth = useBottomSheetStore((state) => { + if (!sheetId) return 0; - const { stackOrder, sheetsById } = state; - const sheetIndex = stackOrder.indexOf(sheetId); + const groupStack = state.stackOrderByGroup[groupId] ?? []; + const sheetIndex = groupStack.indexOf(sheetId); - if (sheetIndex === -1) { - return prevDepthRef.current; - } + if (sheetIndex === -1) return null; let depth = 0; - for (let i = sheetIndex + 1; i < stackOrder.length; i++) { - const id = stackOrder[i]!; - const sheet = sheetsById[id]; + for (let i = sheetIndex + 1; i < groupStack.length; i++) { + const sheet = state.sheetsById[groupStack[i]!]; if ( sheet && - sheet.groupId === groupId && sheet.scaleBackground && sheet.status !== 'closing' && sheet.status !== 'hidden' @@ -91,10 +92,20 @@ function useSheetScaleDepth( } } - prevDepthRef.current = depth; return depth; }); - return result; + + const [heldDepth, setHeldDepth] = useState(0); + const heldDepthRef = useRef(0); + + useEffect(() => { + if (liveDepth !== null && liveDepth !== heldDepthRef.current) { + heldDepthRef.current = liveDepth; + setHeldDepth(liveDepth); + } + }, [liveDepth]); + + return liveDepth ?? heldDepth; } function useScaleAnimatedStyleInternal(scaleDepth: number) { diff --git a/src/useSheetRenderData.ts b/src/useSheetRenderData.ts index d244cca..ed88de9 100644 --- a/src/useSheetRenderData.ts +++ b/src/useSheetRenderData.ts @@ -57,10 +57,13 @@ export function useSheetRenderData(): SheetRenderItem[] { } function getHiddenPersistentSheets( - state: { sheetsById: Record; stackOrder: string[] }, + state: { + sheetsById: Record; + stackOrderByGroup: Record; + }, groupId: string ): SheetRenderItem[] { - const inStack = new Set(state.stackOrder); + const inStack = new Set(state.stackOrderByGroup[groupId] ?? []); return Object.values(state.sheetsById) .filter((sheet) => isHiddenPersistent(sheet, groupId, inStack)) @@ -85,14 +88,17 @@ function isHiddenPersistent( } function getActiveSheets( - state: { sheetsById: Record; stackOrder: string[] }, + state: { + sheetsById: Record; + stackOrderByGroup: Record; + }, groupId: string ): SheetRenderItem[] { - return state.stackOrder - .filter((id) => state.sheetsById[id]?.groupId === groupId) - .map((id, index) => ({ - id, - stackIndex: index, - isActive: true, - })); + // Already scoped to the group, so no filtering is needed — and stackIndex is + // now per-group, which is what the z-index layering wants. + return (state.stackOrderByGroup[groupId] ?? []).map((id, index) => ({ + id, + stackIndex: index, + isActive: true, + })); } diff --git a/src/useEvent.ts b/src/useStableCallback.ts similarity index 51% rename from src/useEvent.ts rename to src/useStableCallback.ts index 379c778..1f80851 100644 --- a/src/useEvent.ts +++ b/src/useStableCallback.ts @@ -3,8 +3,16 @@ import { useCallback, useLayoutEffect, useRef } from 'react'; // biome-ignore lint/suspicious/noExplicitAny: No better alternative available. type CallbackType = (...args: any[]) => any; -// RFC: https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md -export const useEvent = (callback: T) => { +/** + * Stable function identity with an always-fresh closure. + * + * Named `useStableCallback` rather than `useEvent` (the RFC's name) because + * `react-native-reanimated` exports an unrelated `useEvent` for native event + * handlers, and adapters import both. + * + * RFC: https://github.com/reactjs/rfcs/blob/useevent/text/0000-useevent.md + */ +export const useStableCallback = (callback: T) => { const callbackRef = useRef(callback); useLayoutEffect(() => { From 49447f66435a3335b02602a2e8dad5e7153e78f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 19:18:31 +0000 Subject: [PATCH 03/13] =?UTF-8?q?feat!:=20stages=202=20and=203=20=E2=80=94?= =?UTF-8?q?=20consistent=20behaviour,=20then=20API=20cleanup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: removes the deprecated aliases, renames clear() to destroyAll(), narrows isOpen, and moves test helpers to a subpath. See below for the full list. Stage 2 — behavioural consistency (B3, B5, P1, P5, P7, P10, dead code): Backdrops that snapped (B5). animatedIndex was driven continuously by gorhom and swmansion but discretely by the other three adapters, so their backdrops jumped to full opacity on the first frame while the sheet itself was still animating — the same symptom reported for swmansion, baked into the adapters. CustomModalAdapter now derives it from the `progress` it already animates; ReactNativeModalAdapter fades over the modal's own animationInTiming/animationOutTiming; ActionsSheetAdapter springs with the sheet's own open/close spring config. open() reports its outcome (B3). Two guards could silently drop an open — re-opening an active sheet, or opening while another sheet in the group is mid-animation. The caller got an id back either way. open() now returns OpenResult and useBottomSheetManager().open() returns `string | null`, with a __DEV__ warning naming the reason. This is also what lets the ref be registered only on success (B2, stage 1). close() reports whether it closed (P1). requestClose already returned Promise and closeAll returned its promise, but the three main close() calls dropped it — so an onBeforeClose block was invisible unless you reached for the adapter-level API. Adapter authors get the full toolkit (P7). Every built-in adapter uses useSetBackdrop to suppress the shared backdrop; it was never exported, so custom adapters could not match them. Now exported alongside useSheetPreventDismiss. Typed adapters (P10). react-native-modal and react-native-actions-sheet were typed as `[key: string]: unknown`, which disables checking entirely — a prop typo passed silently. Both ship types; they are now devDependencies (as gorhom and swmansion already were) and both adapters extend the real prop types. Also: useBottomSheetStatus accepts BottomSheetPortalId for completion (P5), and the dead code in section 4 of API-REVIEW.md is gone. Stage 3 — API cleanup (P2, P4, P6, P8, P9, W1-W8): - clear() -> destroyAll(). The old name read as tidying up while actually bypassing onBeforeClose and skipping the exit animation. - isOpen now means open, not "open or opening". Added isOpening, isClosing and isVisible for the states it used to conflate. - useBottomSheetStore and the store's state shape are marked @internal; the exported BottomSheetState is narrowed to the fields that are stable. - Test helpers moved to the /testing subpath, with a single resetBottomSheetRegistries() that cannot go stale as registries are added. - Removed: openBottomSheet, clearAll, closeBottomSheet, useBottomSheetState, ModalAdapter, BottomSheetManaged, BottomSheetManagedProps, and the unmarked BottomSheetRef alias. - Local useEvent renamed to useStableCallback, so it no longer collides with reanimated's unrelated useEvent (both are imported by the swmansion adapter); the bottomSheet.store re-export layer is collapsed into ./store; mount() now uses TriggerState like open(); MODE_STATUS_MAP no longer uses null as "no action"; shallow dropped from selectors returning primitives. --- example/babel.config.js | 1 + .../components/BottomSheetDebugMonitor.tsx | 6 +- example/src/components/Sheet.tsx | 10 +- package.json | 7 + src/BottomSheetHost.tsx | 2 +- src/BottomSheetPersistent.tsx | 2 +- src/BottomSheetPortal.tsx | 2 +- src/BottomSheetRef.context.ts | 2 +- src/QueueItem.tsx | 2 +- .../actions-sheet/ActionsSheetAdapter.tsx | 142 ++++++++------ .../custom-modal/CustomModalAdapter.tsx | 17 +- src/adapters/custom-modal/index.ts | 3 - .../gorhom-sheet/GorhomSheetAdapter.tsx | 5 +- src/adapters/gorhom-sheet/index.ts | 5 - src/adapters/index.ts | 6 +- .../ReactNativeModalAdapter.tsx | 176 ++++++++++++------ .../swmansion/SwmansionSheetAdapter.tsx | 2 +- src/bottomSheet.store.ts | 1 - src/bottomSheetCoordinator.ts | 2 +- src/index.tsx | 52 ++++-- src/portalSessionRegistry.ts | 4 - src/testing.ts | 43 +++++ src/useBackHandler.ts | 2 +- src/useBottomSheetContext.ts | 10 +- src/useBottomSheetControl.ts | 2 +- src/useBottomSheetManager.tsx | 18 +- src/useBottomSheetStatus.ts | 31 ++- src/useOnBeforeClose.ts | 2 +- src/useScaleAnimation.ts | 2 +- src/useSheetRenderData.ts | 5 +- src/useTracePropChanges.ts | 27 --- tsconfig.json | 1 + yarn.lock | 2 + 33 files changed, 367 insertions(+), 227 deletions(-) delete mode 100644 src/bottomSheet.store.ts create mode 100644 src/testing.ts delete mode 100644 src/useTracePropChanges.ts diff --git a/example/babel.config.js b/example/babel.config.js index 4f0a13f..b42945a 100644 --- a/example/babel.config.js +++ b/example/babel.config.js @@ -38,6 +38,7 @@ module.exports = function (api) { root, 'src/adapters/swmansion' ), + [`${pkg.name}/testing`]: path.resolve(root, 'src/testing'), }, }, 'subpath-aliases', diff --git a/example/src/components/BottomSheetDebugMonitor.tsx b/example/src/components/BottomSheetDebugMonitor.tsx index e9b22d6..1884117 100644 --- a/example/src/components/BottomSheetDebugMonitor.tsx +++ b/example/src/components/BottomSheetDebugMonitor.tsx @@ -11,10 +11,8 @@ import { Alert, } from 'react-native'; import Clipboard from '@react-native-clipboard/clipboard'; -import { - useBottomSheetStore, - __getAllAnimatedIndexes, -} from 'react-native-bottom-sheet-stack'; +import { useBottomSheetStore } from 'react-native-bottom-sheet-stack'; +import { __getAllAnimatedIndexes } from 'react-native-bottom-sheet-stack/testing'; interface LogEntry { timestamp: number; diff --git a/example/src/components/Sheet.tsx b/example/src/components/Sheet.tsx index a9b3842..184df5d 100644 --- a/example/src/components/Sheet.tsx +++ b/example/src/components/Sheet.tsx @@ -6,7 +6,7 @@ import { import type { BottomSheetMethods } from '@gorhom/bottom-sheet/lib/typescript/types'; import { forwardRef, useCallback, useMemo, type ReactNode } from 'react'; import { View, type StyleProp, type ViewStyle } from 'react-native'; -import { BottomSheetManaged } from '../../../src/adapters/gorhom-sheet'; +import { GorhomSheetAdapter } from '../../../src/adapters/gorhom-sheet'; import { useSafeAreaInsets } from 'react-native-safe-area-context'; import { colors, sharedStyles } from '../styles/theme'; @@ -82,7 +82,7 @@ export const Sheet = forwardRef( if (snapPoints) { return ( - ( children )} - + ); } return ( - ( {scrollable ? {children} : children} - + ); } ); diff --git a/package.json b/package.json index 118ffe9..a9a68a9 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,11 @@ "react-native": "./lib/module/index.js", "default": "./lib/commonjs/index.js" }, + "./testing": { + "types": "./lib/typescript/src/testing.d.ts", + "react-native": "./lib/module/testing.js", + "default": "./lib/commonjs/testing.js" + }, "./gorhom": { "types": "./lib/typescript/src/adapters/gorhom-sheet/index.d.ts", "react-native": "./lib/module/adapters/gorhom-sheet/index.js", @@ -90,8 +95,10 @@ "prettier": "^3.0.3", "react": "19.1.0", "react-native": "0.81.5", + "react-native-actions-sheet": "^10.1.2", "react-native-builder-bob": "^0.40.6", "react-native-gesture-handler": "^2.30.0", + "react-native-modal": "^14.0.0-rc.1", "react-native-reanimated": "^4.2.1", "react-native-safe-area-context": "^5.6.2", "react-native-teleport": "^1.1.7", diff --git a/src/BottomSheetHost.tsx b/src/BottomSheetHost.tsx index 51040e5..a8e83b9 100644 --- a/src/BottomSheetHost.tsx +++ b/src/BottomSheetHost.tsx @@ -1,6 +1,6 @@ import { useEffect } from 'react'; -import { useClearGroup } from './bottomSheet.store'; +import { useClearGroup } from './store'; import { initBottomSheetCoordinator } from './bottomSheetCoordinator'; import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; import { QueueItem } from './QueueItem'; diff --git a/src/BottomSheetPersistent.tsx b/src/BottomSheetPersistent.tsx index 30cb669..8e9996f 100644 --- a/src/BottomSheetPersistent.tsx +++ b/src/BottomSheetPersistent.tsx @@ -9,7 +9,7 @@ import { useSheetExists, useSheetPortalSession, useUnmount, -} from './bottomSheet.store'; +} from './store'; import { BottomSheetDefaultIndexContext } from './BottomSheetDefaultIndex.context'; import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; import { BottomSheetRefContext } from './BottomSheetRef.context'; diff --git a/src/BottomSheetPortal.tsx b/src/BottomSheetPortal.tsx index e64bbf9..ae3f823 100644 --- a/src/BottomSheetPortal.tsx +++ b/src/BottomSheetPortal.tsx @@ -5,7 +5,7 @@ import { StyleSheet } from 'react-native'; import { Portal } from 'react-native-teleport'; import { BottomSheetContext } from './BottomSheet.context'; -import { useSheetPortalSession } from './bottomSheet.store'; +import { useSheetPortalSession } from './store'; import { BottomSheetDefaultIndexContext } from './BottomSheetDefaultIndex.context'; import { BottomSheetRefContext } from './BottomSheetRef.context'; import type { BottomSheetPortalId } from './portal.types'; diff --git a/src/BottomSheetRef.context.ts b/src/BottomSheetRef.context.ts index 339abc6..4546e9b 100644 --- a/src/BottomSheetRef.context.ts +++ b/src/BottomSheetRef.context.ts @@ -3,7 +3,7 @@ import type { SheetRef } from './adapter.types'; /** * Context for passing sheet ref from BottomSheetPersistent/BottomSheetPortal - * to BottomSheetManaged. This allows automatic ref binding without user intervention. + * to the adapter. This allows automatic ref binding without user intervention. */ export const BottomSheetRefContext = createContext(null); diff --git a/src/QueueItem.tsx b/src/QueueItem.tsx index e8cf7e9..1fb351f 100644 --- a/src/QueueItem.tsx +++ b/src/QueueItem.tsx @@ -12,7 +12,7 @@ import { useSheetKeepMounted, useSheetPortalSession, useSheetUsePortal, -} from './bottomSheet.store'; +} from './store'; import { BottomSheetBackdrop } from './BottomSheetBackdrop'; import { removeOnBeforeClose } from './onBeforeCloseRegistry'; import { cleanupSheetRef } from './refsMap'; diff --git a/src/adapters/actions-sheet/ActionsSheetAdapter.tsx b/src/adapters/actions-sheet/ActionsSheetAdapter.tsx index 89b80ca..56b4cc6 100644 --- a/src/adapters/actions-sheet/ActionsSheetAdapter.tsx +++ b/src/adapters/actions-sheet/ActionsSheetAdapter.tsx @@ -1,22 +1,47 @@ import React, { useImperativeHandle, useRef } from 'react'; +import { withSpring } from 'react-native-reanimated'; + +import type { + ActionSheetProps, + ActionSheetRef, +} from 'react-native-actions-sheet'; import type { SheetAdapterRef } from '../../adapter.types'; -import { useSheetPreventDismiss } from '../../bottomSheet.store'; +import { useSheetPreventDismiss } from '../../store'; import { createSheetEventHandlers } from '../../bottomSheetCoordinator'; import { useAdapterRef } from '../../useAdapterRef'; import { useAnimatedIndex } from '../../useAnimatedIndex'; import { useBottomSheetContext } from '../../useBottomSheetContext'; -const ActionSheet = require('react-native-actions-sheet').default; +// Lazy require so the main bundle never loads the library unless this adapter +// is imported (it's an optional peer dependency). +const ActionSheet = require('react-native-actions-sheet') + .default as React.ComponentType< + ActionSheetProps & React.RefAttributes +>; + +/** + * Props for {@link ActionsSheetAdapter}. + * + * Forwards the full prop surface of `react-native-actions-sheet`, except the + * props the stack manager owns: + * + * - `isModal` — forced off; the manager handles the overlay lifecycle. + * - `onOpen` / `onClose` / `onBeforeClose` — consumed by the adapter to report + * lifecycle back to the manager. + */ +export interface ActionsSheetAdapterProps + extends Omit< + ActionSheetProps, + 'isModal' | 'onOpen' | 'onClose' | 'onBeforeClose' | 'children' + > { + children: React.ReactNode; +} /** * Adapter for `react-native-actions-sheet` — a zero-dependency action sheet * with snap points and gesture controls. * - * All ActionSheet props are accepted via spread and forwarded to the - * underlying component. Uses `isModal={false}` internally — the stack - * manager handles the overlay lifecycle. - * * Requires `react-native-actions-sheet` as a peer dependency: * ``` * npm install react-native-actions-sheet @@ -24,62 +49,71 @@ const ActionSheet = require('react-native-actions-sheet').default; * * @see https://github.com/ammarahm-ed/react-native-actions-sheet */ -export interface ActionsSheetAdapterProps { - children: React.ReactNode; - [key: string]: unknown; -} - export const ActionsSheetAdapter = React.forwardRef< SheetAdapterRef, ActionsSheetAdapterProps ->(({ children, ...sheetProps }, forwardedRef) => { - const { id } = useBottomSheetContext(); - const ref = useAdapterRef(forwardedRef); - const animatedIndex = useAnimatedIndex(); - const preventDismiss = useSheetPreventDismiss(id); +>( + ( + { children, openAnimationConfig, closeAnimationConfig, ...sheetProps }, + forwardedRef + ) => { + const { id } = useBottomSheetContext(); + const ref = useAdapterRef(forwardedRef); + const animatedIndex = useAnimatedIndex(); + const preventDismiss = useSheetPreventDismiss(id); - const actionSheetRef = useRef(null); + const actionSheetRef = useRef(null); - const { handleDismiss, handleOpened, handleClosed } = - createSheetEventHandlers(id); + const { handleDismiss, handleOpened, handleClosed } = + createSheetEventHandlers(id); - useImperativeHandle( - ref, - () => ({ - expand: () => actionSheetRef.current?.show(), - close: () => actionSheetRef.current?.hide(), - }), - [] - ); + useImperativeHandle( + ref, + () => ({ + expand: () => actionSheetRef.current?.show(), + close: () => actionSheetRef.current?.hide(), + }), + [] + ); - const onOpen = () => { - animatedIndex.set(0); - handleOpened(); - }; + // Sprung with the sheet's own config rather than set discretely: a discrete + // set puts the manager's backdrop at full opacity on the first frame, ahead + // of the sheet it is meant to be backing. The sheet animates with a spring, + // so the backdrop uses one too — same config, same curve. + // + // onOpen/onClose fire when the sheet *starts* moving, which is what makes + // this work: the fade runs alongside the sheet's own animation. + const onOpen = () => { + animatedIndex.set(withSpring(0, openAnimationConfig)); + handleOpened(); + }; - const onClose = () => { - animatedIndex.set(-1); - handleClosed(); - }; + const onClose = () => { + animatedIndex.set(withSpring(-1, closeAnimationConfig)); + handleClosed(); + }; - return ( - - {children} - - ); -}); + return ( + + {children} + + ); + } +); ActionsSheetAdapter.displayName = 'ActionsSheetAdapter'; diff --git a/src/adapters/custom-modal/CustomModalAdapter.tsx b/src/adapters/custom-modal/CustomModalAdapter.tsx index 5627751..611ca60 100644 --- a/src/adapters/custom-modal/CustomModalAdapter.tsx +++ b/src/adapters/custom-modal/CustomModalAdapter.tsx @@ -3,6 +3,7 @@ import { StyleSheet, type StyleProp, type ViewStyle } from 'react-native'; import Animated, { useAnimatedReaction, useAnimatedStyle, + useDerivedValue, useSharedValue, withTiming, } from 'react-native-reanimated'; @@ -45,16 +46,20 @@ export const CustomModalAdapter = React.forwardRef< expand: () => { setRendered(true); setOpen(true); - animatedIndex.set(0); - }, - close: () => { - setOpen(false); - animatedIndex.set(-1); }, + close: () => setOpen(false), }), - [animatedIndex] + [] ); + // Drive animatedIndex off the same `progress` that animates the modal, so + // the manager's backdrop fades in step with it. Setting it discretely in + // expand/close (as this once did) snapped the backdrop to full opacity on + // the first frame while the modal itself took ANIMATION_DURATION to arrive. + useDerivedValue(() => { + animatedIndex.set(progress.value - 1); + }); + const onAnimationEnd = (value: boolean) => { 'worklet'; if (value) { diff --git a/src/adapters/custom-modal/index.ts b/src/adapters/custom-modal/index.ts index ab0e2e6..c2744f3 100644 --- a/src/adapters/custom-modal/index.ts +++ b/src/adapters/custom-modal/index.ts @@ -2,6 +2,3 @@ export { CustomModalAdapter, type ModalAdapterProps, } from './CustomModalAdapter'; - -/** @deprecated Use `CustomModalAdapter` instead. */ -export { CustomModalAdapter as ModalAdapter } from './CustomModalAdapter'; diff --git a/src/adapters/gorhom-sheet/GorhomSheetAdapter.tsx b/src/adapters/gorhom-sheet/GorhomSheetAdapter.tsx index c1691bd..5f372c1 100644 --- a/src/adapters/gorhom-sheet/GorhomSheetAdapter.tsx +++ b/src/adapters/gorhom-sheet/GorhomSheetAdapter.tsx @@ -8,10 +8,7 @@ import { useAnimatedReaction } from 'react-native-reanimated'; import { scheduleOnRN } from 'react-native-worklets'; import type { SheetAdapterRef } from '../../adapter.types'; -import { - useSetBackdrop, - useSheetPreventDismiss, -} from '../../bottomSheet.store'; +import { useSetBackdrop, useSheetPreventDismiss } from '../../store'; import { createSheetEventHandlers } from '../../bottomSheetCoordinator'; import { useBottomSheetDefaultIndex } from '../../BottomSheetDefaultIndex.context'; import { useAdapterRef } from '../../useAdapterRef'; diff --git a/src/adapters/gorhom-sheet/index.ts b/src/adapters/gorhom-sheet/index.ts index baa6dea..b7f27de 100644 --- a/src/adapters/gorhom-sheet/index.ts +++ b/src/adapters/gorhom-sheet/index.ts @@ -2,8 +2,3 @@ export { GorhomSheetAdapter, type GorhomSheetAdapterProps, } from './GorhomSheetAdapter'; - -/** @deprecated Use `GorhomSheetAdapter` instead. */ -export { GorhomSheetAdapter as BottomSheetManaged } from './GorhomSheetAdapter'; -/** @deprecated Use `GorhomSheetAdapterProps` instead. */ -export { type GorhomSheetAdapterProps as BottomSheetManagedProps } from './GorhomSheetAdapter'; diff --git a/src/adapters/index.ts b/src/adapters/index.ts index a4f2eca..b1244e7 100644 --- a/src/adapters/index.ts +++ b/src/adapters/index.ts @@ -2,11 +2,7 @@ export { GorhomSheetAdapter, type GorhomSheetAdapterProps, } from './gorhom-sheet'; -export { - CustomModalAdapter, - ModalAdapter, - type ModalAdapterProps, -} from './custom-modal'; +export { CustomModalAdapter, type ModalAdapterProps } from './custom-modal'; export { ReactNativeModalAdapter, type ReactNativeModalAdapterProps, diff --git a/src/adapters/react-native-modal/ReactNativeModalAdapter.tsx b/src/adapters/react-native-modal/ReactNativeModalAdapter.tsx index e9c6f6e..b05b3e4 100644 --- a/src/adapters/react-native-modal/ReactNativeModalAdapter.tsx +++ b/src/adapters/react-native-modal/ReactNativeModalAdapter.tsx @@ -1,20 +1,68 @@ import React, { useImperativeHandle, useState } from 'react'; +import { withTiming } from 'react-native-reanimated'; + +import type { ModalProps } from 'react-native-modal'; import type { SheetAdapterRef } from '../../adapter.types'; -import { useSheetPreventDismiss } from '../../bottomSheet.store'; +import { useSheetPreventDismiss } from '../../store'; import { createSheetEventHandlers } from '../../bottomSheetCoordinator'; import { useAdapterRef } from '../../useAdapterRef'; import { useAnimatedIndex } from '../../useAnimatedIndex'; import { useBottomSheetContext } from '../../useBottomSheetContext'; -const RNModal = require('react-native-modal').default; +// Lazy require so the main bundle never loads the library unless this adapter +// is imported (it's an optional peer dependency). +// +// Typed against `Partial`: the library declares most of ModalProps +// as required and supplies them through `defaultProps`, so at a call site every +// one of them is genuinely optional. +const RNModal = require('react-native-modal').default as React.ComponentType< + Partial & { children?: React.ReactNode } +>; + +/** react-native-modal's own defaults, mirrored so the backdrop can match them. */ +const DEFAULT_ANIMATION_IN_TIMING = 300; +const DEFAULT_ANIMATION_OUT_TIMING = 300; + +/** + * Props for {@link ReactNativeModalAdapter}. + * + * Forwards the full prop surface of `react-native-modal`, except the props the + * stack manager owns: + * + * - `isVisible` — the manager drives visibility through the adapter ref. + * - `coverScreen` — forced off so the modal renders as a plain `View` and + * `QueueItem`'s z-index controls stacking. + * - `hasBackdrop` — forced off; the manager's shared `BottomSheetBackdrop` + * provides the overlay. + * - `onModalShow` / `onModalHide` / `onBackButtonPress` / `onSwipeComplete` — + * consumed by the adapter to report lifecycle back to the manager. + */ +export interface ReactNativeModalAdapterProps + // Partial because react-native-modal declares most of `ModalProps` as + // required and fills them from `defaultProps` — as a consumer-facing type + // every one of them is optional. + extends Partial< + Omit< + ModalProps, + | 'isVisible' + | 'coverScreen' + | 'hasBackdrop' + | 'onModalShow' + | 'onModalHide' + | 'onBackButtonPress' + | 'onSwipeComplete' + | 'children' + > + > { + children: React.ReactNode; +} /** * Adapter for `react-native-modal`. * - * All react-native-modal props are accepted via spread and forwarded - * to the underlying component. The adapter sets opinionated defaults - * (swipe-to-dismiss, native driver) that can be overridden. + * The adapter sets opinionated defaults (swipe-to-dismiss, native driver) that + * can be overridden. * * Requires `react-native-modal` as a peer dependency: * ``` @@ -23,61 +71,79 @@ const RNModal = require('react-native-modal').default; * * @see https://github.com/react-native-modal/react-native-modal */ -export interface ReactNativeModalAdapterProps { - children: React.ReactNode; - [key: string]: unknown; -} - export const ReactNativeModalAdapter = React.forwardRef< SheetAdapterRef, ReactNativeModalAdapterProps ->(({ children, ...modalProps }, forwardedRef) => { - const { id } = useBottomSheetContext(); - const ref = useAdapterRef(forwardedRef); - const animatedIndex = useAnimatedIndex(); - const preventDismiss = useSheetPreventDismiss(id); - const [isVisible, setIsVisible] = useState(false); +>( + ( + { + children, + animationInTiming = DEFAULT_ANIMATION_IN_TIMING, + animationOutTiming = DEFAULT_ANIMATION_OUT_TIMING, + ...modalProps + }, + forwardedRef + ) => { + const { id } = useBottomSheetContext(); + const ref = useAdapterRef(forwardedRef); + const animatedIndex = useAnimatedIndex(); + const preventDismiss = useSheetPreventDismiss(id); + const [isVisible, setIsVisible] = useState(false); - const { handleDismiss, handleOpened, handleClosed } = - createSheetEventHandlers(id); + const { handleDismiss, handleOpened, handleClosed } = + createSheetEventHandlers(id); - useImperativeHandle( - ref, - () => ({ - expand: () => { - setIsVisible(true); - animatedIndex.set(0); - }, - close: () => { - setIsVisible(false); - animatedIndex.set(-1); - }, - }), - [animatedIndex] - ); + useImperativeHandle( + ref, + () => ({ + expand: () => { + setIsVisible(true); + // Faded over the modal's own timing rather than set discretely: a + // discrete set puts the manager's backdrop at full opacity on the + // first frame, a whole animation ahead of the modal itself. + animatedIndex.set( + withTiming(0, { + duration: animationInTiming, + }) + ); + }, + close: () => { + setIsVisible(false); + animatedIndex.set( + withTiming(-1, { + duration: animationOutTiming, + }) + ); + }, + }), + [animatedIndex, animationInTiming, animationOutTiming] + ); - return ( - - {children} - - ); -}); + return ( + + {children} + + ); + } +); ReactNativeModalAdapter.displayName = 'ReactNativeModalAdapter'; diff --git a/src/adapters/swmansion/SwmansionSheetAdapter.tsx b/src/adapters/swmansion/SwmansionSheetAdapter.tsx index b6264bd..5065a03 100644 --- a/src/adapters/swmansion/SwmansionSheetAdapter.tsx +++ b/src/adapters/swmansion/SwmansionSheetAdapter.tsx @@ -27,7 +27,7 @@ import type { import type { SheetAdapterRef } from '../../adapter.types'; import { useBottomSheetDefaultIndex } from '../../BottomSheetDefaultIndex.context'; -import { useSheetPreventDismiss } from '../../bottomSheet.store'; +import { useSheetPreventDismiss } from '../../store'; import { createSheetEventHandlers } from '../../bottomSheetCoordinator'; import { useAdapterRef } from '../../useAdapterRef'; import { useAnimatedIndex } from '../../useAnimatedIndex'; diff --git a/src/bottomSheet.store.ts b/src/bottomSheet.store.ts deleted file mode 100644 index d406816..0000000 --- a/src/bottomSheet.store.ts +++ /dev/null @@ -1 +0,0 @@ -export * from './store'; diff --git a/src/bottomSheetCoordinator.ts b/src/bottomSheetCoordinator.ts index 0d71610..e42c502 100644 --- a/src/bottomSheetCoordinator.ts +++ b/src/bottomSheetCoordinator.ts @@ -1,5 +1,5 @@ import type { SheetAdapterEvents } from './adapter.types'; -import { useBottomSheetStore } from './bottomSheet.store'; +import { useBottomSheetStore } from './store'; import { getOnBeforeClose } from './onBeforeCloseRegistry'; import { getSheetRef } from './refsMap'; diff --git a/src/index.tsx b/src/index.tsx index 5c05c4d..77e933a 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -14,12 +14,16 @@ export { // Adapter types export type { SheetAdapterRef, - SheetAdapterRef as BottomSheetRef, SheetAdapterEvents, SheetRef, } from './adapter.types'; -// Adapter utilities (for custom adapter authors) +// --------------------------------------------------------------------------- +// Adapter utilities +// +// Everything a custom adapter needs to reach parity with the built-in ones — +// whatever the shipped adapters use, a third-party one can use too. +// --------------------------------------------------------------------------- export { createSheetEventHandlers, requestClose, @@ -28,20 +32,23 @@ export { export { useAdapterRef } from './useAdapterRef'; export { useAnimatedIndex } from './useAnimatedIndex'; export { useBackHandler } from './useBackHandler'; -export { getAnimatedIndex, setAnimatedIndexValue } from './animatedRegistry'; +/** + * `useSetBackdrop` suppresses the manager's shared backdrop for a sheet — for + * adapters that render their own and would otherwise stack two. + * `useSheetPreventDismiss` reports whether an `onBeforeClose` interceptor is + * blocking dismissal, so the adapter can disable its native gestures. + */ +export { useSetBackdrop, useSheetPreventDismiss } from './store'; // Hooks -export { - useBottomSheetManager, - type CloseAllOptions, -} from './useBottomSheetManager'; +export { useBottomSheetManager } from './useBottomSheetManager'; +export type { CloseAllOptions } from './useBottomSheetManager'; export { useBottomSheetControl, type UseBottomSheetControlReturn, } from './useBottomSheetControl'; export { useBottomSheetContext, - useBottomSheetState, type UseBottomSheetContextReturn, } from './useBottomSheetContext'; export { @@ -55,25 +62,30 @@ export type { ScaleConfig, ScaleAnimationConfig } from './useScaleAnimation'; export type { BottomSheetStatus, OpenMode, - BottomSheetState, -} from './bottomSheet.store'; + OpenResult, + OpenRejectionReason, + PublicBottomSheetState as BottomSheetState, +} from './store'; export type { BottomSheetPortalRegistry, BottomSheetPortalId, BottomSheetPortalParams, } from './portal.types'; -export { useBottomSheetStore } from './bottomSheet.store'; - // onBeforeClose registry export type { OnBeforeCloseCallback } from './onBeforeCloseRegistry'; export { setOnBeforeClose, removeOnBeforeClose } from './onBeforeCloseRegistry'; -// Testing utilities (internal use) -export { __resetSheetRefs } from './refsMap'; -export { - __resetAnimatedIndexes, - __getAllAnimatedIndexes, -} from './animatedRegistry'; -export { __resetPortalSessions } from './portalSessionRegistry'; -export { __resetOnBeforeClose } from './onBeforeCloseRegistry'; +/** + * Direct access to the Zustand store. + * + * @internal Not covered by semver. The state shape and the action set are + * implementation details — `stackOrderByGroup`, `content`, `portalSession` and + * the lifecycle actions (`markOpen`, `finishClosing`, `mount`, `unmount`) exist + * to serve the coordinator and can change without a major bump. Prefer + * `useBottomSheetStatus`, `useBottomSheetContext` and `useBottomSheetControl`. + */ +export { useBottomSheetStore } from './store'; + +// Test helpers live on the `/testing` subpath, so they stay out of the +// production bundle. See src/testing.ts. diff --git a/src/portalSessionRegistry.ts b/src/portalSessionRegistry.ts index 389a591..3702b3a 100644 --- a/src/portalSessionRegistry.ts +++ b/src/portalSessionRegistry.ts @@ -12,10 +12,6 @@ export function getNextPortalSession(sheetId: string): number { return next; } -export function getCurrentPortalSession(sheetId: string): number | undefined { - return portalSessionRegistry.get(sheetId); -} - /** * Reset all portal sessions. Useful for testing. * @internal diff --git a/src/testing.ts b/src/testing.ts new file mode 100644 index 0000000..0b8f991 --- /dev/null +++ b/src/testing.ts @@ -0,0 +1,43 @@ +/** + * Test-only helpers. + * + * Shipped on a separate subpath so they stay out of the production bundle: + * + * ```ts + * import { resetBottomSheetRegistries } from 'react-native-bottom-sheet-stack/testing'; + * + * beforeEach(resetBottomSheetRegistries); + * ``` + */ +import { + __resetAnimatedIndexes, + __getAllAnimatedIndexes, +} from './animatedRegistry'; +import { __resetOnBeforeClose } from './onBeforeCloseRegistry'; +import { __resetPortalSessions } from './portalSessionRegistry'; +import { __resetSheetRefs } from './refsMap'; +import { useBottomSheetStore } from './store'; + +/** + * Clears every module-level registry **and** the store. + * + * The registries outlive React (they are module state), so a test that opens a + * sheet leaves refs, animated values, portal sessions and interceptors behind + * for the next one. Call this between tests rather than resetting each registry + * by hand — it is one call that cannot go out of date as registries are added. + */ +export function resetBottomSheetRegistries(): void { + useBottomSheetStore.getState().clearAll(); + __resetSheetRefs(); + __resetAnimatedIndexes(); + __resetPortalSessions(); + __resetOnBeforeClose(); +} + +export { + __resetSheetRefs, + __resetAnimatedIndexes, + __getAllAnimatedIndexes, + __resetPortalSessions, + __resetOnBeforeClose, +}; diff --git a/src/useBackHandler.ts b/src/useBackHandler.ts index 7b73312..0725ec5 100644 --- a/src/useBackHandler.ts +++ b/src/useBackHandler.ts @@ -1,7 +1,7 @@ import { useEffect } from 'react'; import { BackHandler } from 'react-native'; -import { useIsTopmostAndOpen } from './bottomSheet.store'; +import { useIsTopmostAndOpen } from './store'; /** * Manages Android hardware back button for a sheet. diff --git a/src/useBottomSheetContext.ts b/src/useBottomSheetContext.ts index 7fa0194..9e5aee2 100644 --- a/src/useBottomSheetContext.ts +++ b/src/useBottomSheetContext.ts @@ -3,7 +3,7 @@ import { useSheetParams, useSheetPreventDismiss, useStartClosing, -} from './bottomSheet.store'; +} from './store'; import { requestClose } from './bottomSheetCoordinator'; import type { BottomSheetPortalId, @@ -38,8 +38,6 @@ export interface UseBottomSheetContextReturn { * Useful for force-closing from within onBeforeClose confirmation flows. */ forceClose: () => void; - /** @deprecated Use `close` instead */ - closeBottomSheet: () => void; } /** Without generic - params typed as unknown */ @@ -74,11 +72,5 @@ export function useBottomSheetContext< preventDismiss, close, forceClose, - closeBottomSheet: close, }; } - -/** - * @deprecated Use `useBottomSheetContext` instead - */ -export const useBottomSheetState = useBottomSheetContext; diff --git a/src/useBottomSheetControl.ts b/src/useBottomSheetControl.ts index 1bc7d5f..93ca663 100644 --- a/src/useBottomSheetControl.ts +++ b/src/useBottomSheetControl.ts @@ -1,7 +1,7 @@ import React from 'react'; import type { SheetAdapterRef } from './adapter.types'; -import { useOpen, useUpdateParams, type OpenMode } from './bottomSheet.store'; +import { useOpen, useUpdateParams, type OpenMode } from './store'; import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; import { closeAllAnimated, requestClose } from './bottomSheetCoordinator'; import type { diff --git a/src/useBottomSheetManager.tsx b/src/useBottomSheetManager.tsx index 5b0084d..d2e840e 100644 --- a/src/useBottomSheetManager.tsx +++ b/src/useBottomSheetManager.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { useOpen, useClearGroup, type OpenMode } from './bottomSheet.store'; +import { useOpen, useClearGroup, type OpenMode } from './store'; import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; import type { SheetAdapterRef } from './adapter.types'; import { closeAllAnimated, requestClose } from './bottomSheetCoordinator'; @@ -83,7 +83,15 @@ export const useBottomSheetManager = () => { return closeAllAnimated(groupId, options); }; - const clear = () => { + /** + * Removes every sheet in the group from the store immediately. + * + * This is a teardown primitive, not a way to close sheets: there is no exit + * animation and **`onBeforeClose` interceptors do not run**, so a sheet + * guarding unsaved work is discarded without asking. Use {@link closeAll} for + * anything user-facing. + */ + const destroyAll = () => { const groupId = bottomSheetManagerContext?.groupId || 'default'; storeClearGroup(groupId); }; @@ -92,10 +100,6 @@ export const useBottomSheetManager = () => { open: openBottomSheet, close, closeAll, - clear, - /** @deprecated Use `open` instead */ - openBottomSheet, - /** @deprecated Use `clear` instead */ - clearAll: clear, + destroyAll, }; }; diff --git a/src/useBottomSheetStatus.ts b/src/useBottomSheetStatus.ts index 5a7af8d..5a00be3 100644 --- a/src/useBottomSheetStatus.ts +++ b/src/useBottomSheetStatus.ts @@ -1,15 +1,40 @@ -import { useSheetStatus, type BottomSheetStatus } from './bottomSheet.store'; +import { useSheetStatus, type BottomSheetStatus } from './store'; +import type { BottomSheetPortalId } from './portal.types'; export interface UseBottomSheetStatusReturn { + /** The sheet's status, or `null` when the store has no record of it. */ status: BottomSheetStatus | null; + /** Fully open and interactive. Does **not** include the opening animation. */ isOpen: boolean; + /** Animating in. */ + isOpening: boolean; + /** Animating out. */ + isClosing: boolean; + /** + * On screen in any form — opening, open, or closing. This is the one to use + * for "should I render something alongside the sheet". + */ + isVisible: boolean; } -export function useBottomSheetStatus(id: string): UseBottomSheetStatusReturn { +/** + * Observes a sheet's status from outside the sheet. + * + * Works for every kind of sheet: registered portal and persistent IDs get + * completion from the portal registry, and the random IDs returned by + * `useBottomSheetManager().open()` are accepted just as well. + */ +export function useBottomSheetStatus( + id: BottomSheetPortalId | (string & {}) +): UseBottomSheetStatusReturn { const status = useSheetStatus(id) ?? null; return { status, - isOpen: status === 'open' || status === 'opening', + isOpen: status === 'open', + isOpening: status === 'opening', + isClosing: status === 'closing', + isVisible: + status === 'open' || status === 'opening' || status === 'closing', }; } diff --git a/src/useOnBeforeClose.ts b/src/useOnBeforeClose.ts index b5d76de..20fd4ae 100644 --- a/src/useOnBeforeClose.ts +++ b/src/useOnBeforeClose.ts @@ -1,7 +1,7 @@ import { useEffect } from 'react'; import { useMaybeBottomSheetContext } from './BottomSheet.context'; -import { useSetPreventDismiss } from './bottomSheet.store'; +import { useSetPreventDismiss } from './store'; import type { OnBeforeCloseCallback } from './onBeforeCloseRegistry'; import { removeOnBeforeClose, setOnBeforeClose } from './onBeforeCloseRegistry'; import { useStableCallback } from './useStableCallback'; diff --git a/src/useScaleAnimation.ts b/src/useScaleAnimation.ts index aecc764..9258dd4 100644 --- a/src/useScaleAnimation.ts +++ b/src/useScaleAnimation.ts @@ -7,7 +7,7 @@ import { type WithSpringConfig, type WithTimingConfig, } from 'react-native-reanimated'; -import { useBottomSheetStore } from './bottomSheet.store'; +import { useBottomSheetStore } from './store'; import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; export type ScaleAnimationConfig = diff --git a/src/useSheetRenderData.ts b/src/useSheetRenderData.ts index ed88de9..40eebc8 100644 --- a/src/useSheetRenderData.ts +++ b/src/useSheetRenderData.ts @@ -1,7 +1,4 @@ -import { - useBottomSheetStore, - type BottomSheetState, -} from './bottomSheet.store'; +import { useBottomSheetStore, type BottomSheetState } from './store'; import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; export interface SheetRenderItem { diff --git a/src/useTracePropChanges.ts b/src/useTracePropChanges.ts deleted file mode 100644 index 0056100..0000000 --- a/src/useTracePropChanges.ts +++ /dev/null @@ -1,27 +0,0 @@ -import { useEffect, useRef } from 'react'; - -export function useTracePropChanges( - componentName: string, - props: Record -) { - const prevProps = useRef(props); - - useEffect(() => { - const allKeys = Object.keys({ ...props, ...prevProps.current }); - allKeys.forEach((key) => { - if (prevProps.current[key] !== props[key]) { - console.log(`[${componentName}] Prop '${key}' changed.`); - if ( - typeof props[key] === 'object' || - typeof props[key] === 'function' - ) { - console.log( - `[${componentName}] New instance detected for prop '${key}'.` - ); - } - } - }); - - prevProps.current = props; - }); -} diff --git a/tsconfig.json b/tsconfig.json index 46c6c93..b4fb362 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -4,6 +4,7 @@ "rootDir": ".", "paths": { "react-native-bottom-sheet-stack": ["./src/index"], + "react-native-bottom-sheet-stack/testing": ["./src/testing"], "react-native-bottom-sheet-stack/gorhom": ["./src/adapters/gorhom-sheet/index"], "react-native-bottom-sheet-stack/react-native-modal": ["./src/adapters/react-native-modal/index"], "react-native-bottom-sheet-stack/actions-sheet": ["./src/adapters/actions-sheet/index"], diff --git a/yarn.lock b/yarn.lock index d41707b..72a6815 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12210,8 +12210,10 @@ __metadata: prettier: ^3.0.3 react: 19.1.0 react-native: 0.81.5 + react-native-actions-sheet: ^10.1.2 react-native-builder-bob: ^0.40.6 react-native-gesture-handler: ^2.30.0 + react-native-modal: ^14.0.0-rc.1 react-native-reanimated: ^4.2.1 react-native-safe-area-context: ^5.6.2 react-native-teleport: ^1.1.7 From 9ad006ca21bb277e394a85c81efed2d56ba72791 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 19:21:17 +0000 Subject: [PATCH 04/13] docs: bring guides in line with the reworked API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updates the docs site, README and CLAUDE.md for the stage 2 and 3 changes: the group-keyed stack, open() returning null on rejection, close() returning Promise, destroyAll() replacing clear(), the narrowed isOpen plus the new isOpening/isClosing/isVisible flags, the narrowed public BottomSheetState, and the /testing subpath. Removes the deprecated-alias tables and the BottomSheetManaged/ModalAdapter re-export notes, since those aliases are gone. Adds a section to custom-adapters.md covering useSetBackdrop and useSheetPreventDismiss, which are now exported — the built-in adapters have always used them, so the guide was describing an adapter you could not actually write. CLAUDE.md gains an explicit warning that the stack is keyed by group and that flattening it inside a store action or selector reintroduces the isolation bug the shape exists to prevent. --- CLAUDE.md | 61 ++++++++++++++++----------- docs/docs/api/components.md | 4 +- docs/docs/api/hooks.md | 54 +++++++++++++++--------- docs/docs/api/types.md | 18 +++++--- docs/docs/built-in-adapters/gorhom.md | 4 +- docs/docs/custom-adapters.md | 17 ++++++++ docs/docs/getting-started.md | 2 +- 7 files changed, 102 insertions(+), 58 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5b0d684..a46e6d7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,14 +107,16 @@ A library-agnostic stack manager for bottom sheets and modals in React Native. P ### Core State Management -#### `bottomSheet.store.ts` - Central Zustand Store +#### `store/` - Central Zustand Store **Purpose**: Single source of truth for all sheet state and stack ordering. +Split into `store.ts` (actions), `hooks.ts` (selectors), `helpers.ts` (pure +stack operations) and `types.ts`. Import from `./store`. **State Structure**: ```typescript interface BottomSheetStoreState { - sheetsById: Record; // All sheets by ID - stackOrder: string[]; // Visible sheet IDs in order + sheetsById: Record; // All sheets by ID + stackOrderByGroup: Record; // Visible IDs, per group } interface BottomSheetState { @@ -129,8 +131,17 @@ interface BottomSheetState { } ``` +**CRITICAL — the stack is keyed by group.** There is no global `stackOrder`. +Every stack operation takes a *single group's* array, which is what makes group +isolation structural rather than a filter someone has to remember. Reaching for +`Object.values(stackOrderByGroup).flat()` inside a store action or selector +re-introduces the exact bug this shape exists to prevent (`switch`/`replace` in +one group closing a sheet in another). + **Key Actions**: -- `open(sheet, mode)` - Opens sheet with navigation mode +- `open(sheet, mode)` - Opens sheet with navigation mode. Returns `OpenResult` + (`{ opened: false, reason }` when the sheet is already active or the group is + mid-animation) — never silently drops the request. - `markOpen(id)` - Transitions 'opening' → 'open' - `startClosing(id)` - Initiates close animation - `finishClosing(id)` - Completes close (hides if keepMounted, removes otherwise) @@ -294,7 +305,7 @@ Creates cascading scale effect for nested sheets. **Render Order**: 1. Hidden persistent sheets (keepMounted=true, not in stack) -2. Active sheets (in stackOrder) +2. Active sheets (in its group's stack) This prevents React from unmounting/remounting during state transitions. @@ -509,20 +520,20 @@ open(); // Reopens with previous state intact **Data Flow**: 1. On component mount: `mount()` action creates sheet with `status: 'hidden'` -2. Sheet exists in `sheetsById` but NOT in `stackOrder` -3. On `open()`: Sheet added to `stackOrder`, status → 'opening' -4. On close: Status → 'hidden', removed from `stackOrder` but KEPT in `sheetsById` +2. Sheet exists in `sheetsById` but NOT in its group's stack +3. On `open()`: Sheet added to its group's stack, status → 'opening' +4. On close: Status → 'hidden', removed from its group's stack but KEPT in `sheetsById` 5. Content stays mounted (just hidden), state preserved 6. On component unmount: `unmount()` removes from store completely **Lifecycle Diagram**: ``` -Component Mount → store.mount() → status: 'hidden' (in sheetsById, not in stackOrder) +Component Mount → store.mount() → status: 'hidden' (in sheetsById, not in its group's stack) │ open() called │ ▼ - status: 'opening' (added to stackOrder) + status: 'opening' (added to its group's stack) │ animation done │ @@ -533,7 +544,7 @@ Component Mount → store.mount() → status: 'hidden' (in sheetsById, not in st │ ▼ status: 'closing' → 'hidden' - (removed from stackOrder, kept in sheetsById) + (removed from its group's stack, kept in sheetsById) Content stays mounted! State preserved! │ open() again @@ -563,24 +574,24 @@ Component Mount → store.mount() → status: 'hidden' (in sheetsById, not in st INLINE MODE (useBottomSheetManager): ┌─────────────────────────────────────────────────────┐ │ sheetsById: { 'abc123': { content: , ... } } │ -│ stackOrder: ['abc123'] │ +│ stackOrderByGroup: { default: ['abc123'] } │ └─────────────────────────────────────────────────────┘ After close: Sheet DELETED from sheetsById PORTAL MODE (BottomSheetPortal): ┌─────────────────────────────────────────────────────┐ │ sheetsById: { 'user-sheet': { usePortal: true } } │ -│ stackOrder: ['user-sheet'] │ +│ stackOrderByGroup: { default: ['user-sheet'] } │ └─────────────────────────────────────────────────────┘ After close: Sheet DELETED from sheetsById PERSISTENT MODE (BottomSheetPersistent): ┌──────────────────────────────────────────────────────────────────┐ │ sheetsById: { 'scanner': { usePortal: true, keepMounted: true } }│ -│ stackOrder: ['scanner'] │ +│ stackOrderByGroup: { default: ['scanner'] } │ └──────────────────────────────────────────────────────────────────┘ After close: Sheet KEPT in sheetsById with status: 'hidden' - Removed from stackOrder only + Removed from its group's stack only ``` --- @@ -668,8 +679,8 @@ Multiple `BottomSheetManagerProvider` instances can run independently: ``` Each group: -- Has its own stackOrder -- Sheets filtered by groupId in coordinator +- Has its own entry in `stackOrderByGroup` +- Stack is keyed by group, so no filtering is needed anywhere - `clearGroup(groupId)` clears only that group --- @@ -701,12 +712,12 @@ Each group: ## Testing Utilities ```typescript -import { __resetSheetRefs, __resetAnimatedIndexes } from 'react-native-bottom-sheet-stack'; +import { resetBottomSheetRegistries } from 'react-native-bottom-sheet-stack/testing'; -beforeEach(() => { - __resetSheetRefs(); - __resetAnimatedIndexes(); -}); +// Clears the store and every module-level registry (refs, animated values, +// portal sessions, onBeforeClose). One call, so it cannot go stale as +// registries are added. +beforeEach(resetBottomSheetRegistries); ``` --- @@ -766,7 +777,7 @@ open({ mode: 'replace' }); ``` src/ ├── index.tsx # Public exports (no 3rd-party adapter deps) -├── bottomSheet.store.ts # Zustand store (state + actions) +├── store/ # Zustand store (store/hooks/helpers/types) ├── bottomSheetCoordinator.ts # Store ↔ adapter sync ├── refsMap.ts # Global sheet refs registry ├── animatedRegistry.ts # Global animated values registry @@ -794,7 +805,7 @@ src/ ├── useBackHandler.ts # Android back button handler ├── useScaleAnimation.ts # Scale animation hooks ├── useSheetRenderData.ts # Render order computation hook -├── useEvent.ts # Stable callback utility +├── useStableCallback.ts # Stable callback utility (RFC useEvent) │ └── adapters/ # Each adapter is a separate subpath export ├── gorhom-sheet/ # → 'react-native-bottom-sheet-stack/gorhom' @@ -890,7 +901,7 @@ import { ActionsSheetAdapter } from 'react-native-bottom-sheet-stack/actions-she import { SwmansionSheetAdapter } from 'react-native-bottom-sheet-stack/swmansion'; ``` -**Backward compatibility**: `BottomSheetManaged` and `BottomSheetManagedProps` are available as deprecated re-exports from the gorhom subpath. +**Deprecated aliases were removed in 2.0**: use `GorhomSheetAdapter` / `GorhomSheetAdapterProps`, `CustomModalAdapter`, `useBottomSheetContext`, `open`, `close` and `destroyAll` directly. **Example app (monorepo dev)**: RNBB's `babel-plugin-module-resolver` alias breaks subpath imports. The example's `babel.config.js` adds a separate module-resolver plugin with explicit subpath aliases that runs before RNBB's override. Consumer apps do NOT need this — Metro reads `exports` from package.json directly. diff --git a/docs/docs/api/components.md b/docs/docs/api/components.md index c1ea0c2..1d3f7ca 100644 --- a/docs/docs/api/components.md +++ b/docs/docs/api/components.md @@ -72,9 +72,7 @@ Adapters with 3rd-party dependencies are shipped as separate subpath exports: | `ActionsSheetAdapter` | `react-native-bottom-sheet-stack/actions-sheet` | `react-native-actions-sheet` | [ActionsSheetAdapter](/built-in-adapters/actions-sheet) | | `SwmansionSheetAdapter` | `react-native-bottom-sheet-stack/swmansion` | `@swmansion/react-native-bottom-sheet` | [SwmansionSheetAdapter](/built-in-adapters/swmansion) | -:::tip -`BottomSheetManaged` is available as a deprecated re-export from `react-native-bottom-sheet-stack/gorhom` for backward compatibility. -::: +:::tip::: See [Library-Agnostic Architecture](/adapters) for how adapters work, or [Building Custom Adapters](/custom-adapters) to create your own. diff --git a/docs/docs/api/hooks.md b/docs/docs/api/hooks.md index 2e94634..30081cd 100644 --- a/docs/docs/api/hooks.md +++ b/docs/docs/api/hooks.md @@ -21,17 +21,17 @@ Hooks are divided into two categories based on where they can be used: Main hook for opening and managing bottom sheets imperatively. ```tsx -const { open, close, closeAll, clear } = useBottomSheetManager(); +const { open, close, closeAll, destroyAll } = useBottomSheetManager(); ``` ### Returns | Property | Type | Description | |----------|------|-------------| -| `open` | `(content, options?) => string` | Opens a bottom sheet and returns its ID | -| `close` | `(id: string) => void` | Closes a specific sheet by ID | +| `open` | `(content, options?) => string \| null` | Opens a bottom sheet and returns its ID, or `null` if the store declined | +| `close` | `(id: string) => Promise` | Closes a specific sheet by ID. Resolves `false` if an interceptor blocked it | | `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | -| `clear` | `() => void` | Removes all sheets immediately (no animation) | +| `destroyAll` | `() => void` | Removes all sheets immediately — no animation, **bypasses `onBeforeClose`** | ### closeAll Options @@ -71,12 +71,24 @@ open(, { | `scaleBackground` | `boolean` | `false` | Enable background scaling | | `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. Built-in adapters set this automatically when you give them their own backdrop (e.g. a custom gorhom `backdropComponent`), so you rarely set it by hand. | -### Deprecated Aliases +`open()` returns the sheet's ID, or **`null`** when the store declined to open it — because the sheet is already on the stack, or another sheet in the group is still animating open. A dev-mode warning explains which. -| Deprecated | Use Instead | -|------------|-------------| -| `openBottomSheet` | `open` | -| `clearAll` | `clear` | +```tsx +const id = open(); +if (id === null) { + // Not opened. Nothing to close, nothing to track. +} +``` + +### `destroyAll()` vs `closeAll()` + +| | `closeAll()` | `destroyAll()` | +|---|---|---| +| Animation | staggered cascade | none | +| `onBeforeClose` | respected | **bypassed** | +| Returns | `Promise` | `void` | + +`destroyAll()` is a teardown primitive — it drops every sheet in the group from the store immediately, without asking an interceptor that may be guarding unsaved work. Use `closeAll()` for anything user-facing. --- @@ -113,16 +125,9 @@ console.log(params.userId); // type-safe: string | `id` | `string` | Current sheet's ID | | `params` | `BottomSheetPortalParams` or `unknown` | Type-safe params when generic provided | | `preventDismiss` | `boolean` | Whether dismissal is currently blocked for this sheet (set via `useOnBeforeClose`). Useful for UI that should reflect it — e.g. hiding a grab handle. | -| `close` | `() => void` | Closes this sheet (respects `useOnBeforeClose`) | +| `close` | `() => Promise` | Closes this sheet (respects `useOnBeforeClose`). Resolves `true` once it is closing, `false` if an interceptor blocked it or there was nothing to close. | | `forceClose` | `() => void` | Closes this sheet immediately, bypassing any `useOnBeforeClose` interceptor | -### Deprecated Aliases - -| Deprecated | Use Instead | -|------------|-------------| -| `useBottomSheetState` | `useBottomSheetContext` | -| `closeBottomSheet` | `close` | - --- ## useBottomSheetControl @@ -148,7 +153,7 @@ const { open, close, closeAll, updateParams, resetParams } = useBottomSheetContr | Property | Type | Description | |----------|------|-------------| | `open` | `(options?) => void` | Opens the sheet | -| `close` | `() => void` | Closes the sheet (respects `useOnBeforeClose`) | +| `close` | `() => Promise` | Closes the sheet (respects `useOnBeforeClose`). Resolves `false` if an interceptor blocked it | | `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | | `updateParams` | `(params) => void` | Updates the sheet's params | | `resetParams` | `() => void` | Resets params to `undefined` | @@ -175,6 +180,8 @@ open({ | `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. Built-in adapters set this automatically when given their own native backdrop/scrim, so you rarely set it by hand. | | `params` | `BottomSheetPortalParams` | - | Type-safe params | +`useBottomSheetManager().open()` also accepts `params` now, so inline sheets can read them from `useBottomSheetContext()` just like portal sheets. + --- ## useBottomSheetStatus @@ -200,14 +207,21 @@ const { status, isOpen } = useBottomSheetStatus(sheetId); | Parameter | Type | Description | |-----------|------|-------------| -| `id` | `string` | The sheet ID to observe (portal ID or inline sheet ID) | +| `id` | `BottomSheetPortalId \| (string & {})` | The sheet ID to observe. Registered portal IDs get completion; the random IDs from `open()` are accepted too | ### Returns | Property | Type | Description | |----------|------|-------------| | `status` | `BottomSheetStatus \| null` | Current status or `null` if never opened | -| `isOpen` | `boolean` | `true` if status is `'open'` or `'opening'` | +| `isOpen` | `boolean` | Fully open and interactive — **not** during the opening animation | +| `isOpening` | `boolean` | Animating in | +| `isClosing` | `boolean` | Animating out | +| `isVisible` | `boolean` | On screen in any form: opening, open, or closing | + +:::warning `isOpen` narrowed in 2.0 +It used to be `true` during the opening animation as well. If you were using it to mean "on screen", switch to `isVisible`. +::: ### Status Values diff --git a/docs/docs/api/types.md b/docs/docs/api/types.md index a48007a..125d0d3 100644 --- a/docs/docs/api/types.md +++ b/docs/docs/api/types.md @@ -41,22 +41,23 @@ type OpenMode = 'push' | 'switch' | 'replace'; ### BottomSheetState -Full state object for a bottom sheet. +The stable, public part of a sheet's state. ```tsx interface BottomSheetState { id: string; groupId: string; status: BottomSheetStatus; - content: ReactNode; - scaleBackground?: boolean; - usePortal?: boolean; params?: Record; + scaleBackground?: boolean; keepMounted?: boolean; - preventDismiss?: boolean; } ``` +:::note Narrowed in 2.0 +`content`, `usePortal`, `portalSession` and `preventDismiss` were removed from this type. They are store plumbing — read `preventDismiss` from `useBottomSheetContext()` instead. +::: + | Property | Type | Description | |----------|------|-------------| | `keepMounted` | `boolean` | When `true`, sheet stays in store after close (persistent mode) | @@ -252,7 +253,7 @@ interface UseBottomSheetContextReturn { id: string; params: TParams; preventDismiss: boolean; - close: () => void; + close: () => Promise; forceClose: () => void; } ``` @@ -266,7 +267,12 @@ Return type of `useBottomSheetStatus` hook. ```tsx interface UseBottomSheetStatusReturn { status: BottomSheetStatus | null; + /** Fully open and interactive — not during the opening animation. */ isOpen: boolean; + isOpening: boolean; + isClosing: boolean; + /** On screen in any form: opening, open, or closing. */ + isVisible: boolean; } ``` diff --git a/docs/docs/built-in-adapters/gorhom.md b/docs/docs/built-in-adapters/gorhom.md index 945dd9f..1827e11 100644 --- a/docs/docs/built-in-adapters/gorhom.md +++ b/docs/docs/built-in-adapters/gorhom.md @@ -2,9 +2,7 @@ The default adapter. Wraps `@gorhom/bottom-sheet` to provide feature-rich bottom sheets with snap points, spring animations, and swipe gestures. -:::tip -`BottomSheetManaged` is available as a deprecated re-export from the same subpath for backward compatibility. -::: +:::tip::: ## Installation diff --git a/docs/docs/custom-adapters.md b/docs/docs/custom-adapters.md index d94b0e4..a077935 100644 --- a/docs/docs/custom-adapters.md +++ b/docs/docs/custom-adapters.md @@ -248,6 +248,23 @@ const onIndexChange = (i: number) => { This is exactly how [`SwmansionSheetAdapter`](/built-in-adapters/swmansion) bridges Software Mansion's native sheet. When the library also reports a continuous position (e.g. `onPositionChange`), interpolate it into `animatedIndex` (`[-1, 0]`) for a smooth backdrop fade. +### Suppressing the manager backdrop + +If your adapter renders a backdrop of its own, suppress the manager's shared one so the two don't stack into a double-dark overlay: + +```tsx +import { useSetBackdrop, useSheetPreventDismiss } from 'react-native-bottom-sheet-stack'; + +const setBackdrop = useSetBackdrop(); +useEffect(() => { + if (!hasOwnBackdrop) return; + setBackdrop(id, false); + return () => setBackdrop(id, true); +}, [id, hasOwnBackdrop, setBackdrop]); +``` + +`useSheetPreventDismiss(id)` reports whether a `useOnBeforeClose` interceptor is currently blocking dismissal, so you can disable your library's native swipe/tap gestures while it is. + ### Libraries Without Separate Dismiss/Close Phases Some libraries fire a single `onClose` for both user dismissal and animation completion. In that case, call both: diff --git a/docs/docs/getting-started.md b/docs/docs/getting-started.md index 6a5c736..66eab8f 100644 --- a/docs/docs/getting-started.md +++ b/docs/docs/getting-started.md @@ -24,7 +24,7 @@ Install only the dependencies for the adapter(s) you plan to use: # For GorhomSheetAdapter (default bottom sheet adapter) yarn add @gorhom/bottom-sheet react-native-gesture-handler -# For ModalAdapter — no extra dependencies (uses React Native's built-in Modal) +# For CustomModalAdapter — no extra dependencies # For ReactNativeModalAdapter yarn add react-native-modal From 9f71d99fd56528f97874997a1c9070add7158250 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 20:04:00 +0000 Subject: [PATCH 05/13] fix(example): select from the store instead of subscribing to all of it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debug monitor called useBottomSheetStore() with no selector, so it re-rendered on every store write regardless of what changed, and then flattened stackOrderByGroup in the component body — allocating a new array each render. Both are now selectors with `shallow`, which the flatten actually requires: under the default reference check a freshly allocated array reads as changed on every store write, so an unguarded selector would be worse than the whole-store subscription it replaces. Worth noting for anything else added here: the example app is not compiled with React Compiler. The plugin is declared in the root babel.config.js, which covers src/ only — example/babel.config.js does not include it, and builder-bob's getConfig does not add it. Nothing in the example memoizes itself. --- .../src/components/BottomSheetDebugMonitor.tsx | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/example/src/components/BottomSheetDebugMonitor.tsx b/example/src/components/BottomSheetDebugMonitor.tsx index 1884117..3576868 100644 --- a/example/src/components/BottomSheetDebugMonitor.tsx +++ b/example/src/components/BottomSheetDebugMonitor.tsx @@ -12,6 +12,7 @@ import { } from 'react-native'; import Clipboard from '@react-native-clipboard/clipboard'; import { useBottomSheetStore } from 'react-native-bottom-sheet-stack'; +import { shallow } from 'zustand/shallow'; import { __getAllAnimatedIndexes } from 'react-native-bottom-sheet-stack/testing'; interface LogEntry { @@ -122,9 +123,19 @@ function pollAnimatedIndexValues() { export function BottomSheetDebugMonitor() { const [modalVisible, setModalVisible] = useState(false); const [refreshKey, setRefreshKey] = useState(0); - const { sheetsById, stackOrderByGroup } = useBottomSheetStore(); + const sheetsById = useBottomSheetStore((state) => state.sheetsById, shallow); // Flattened for display only — the store keys the stack by group. - const stackOrder = Object.values(stackOrderByGroup).flat(); + // + // Both selectors take `shallow`, and that is load-bearing here rather than + // decoration: the flatten allocates a fresh array on every call, so with the + // default reference check Zustand would treat every store write as a change + // and re-render this monitor constantly. Nothing in this file is memoized — + // the example app does not run React Compiler (the plugin lives in the root + // babel.config.js, which only covers src/). + const stackOrder = useBottomSheetStore( + (state) => Object.values(state.stackOrderByGroup).flat(), + shallow + ); const pan = useRef(new Animated.ValueXY({ x: 20, y: 100 })).current; From f68aba10d5584a7ac13659f1f9301eee8977d7db Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 20:06:57 +0000 Subject: [PATCH 06/13] docs: correct the API review status section The status claimed W1-W8 were done. W1 (context hook naming) and W4 (the discriminated open() payload) were not touched, so the claim was wrong. Also records what this work introduced rather than fixed: open() reports rejection through useBottomSheetManager but not useBottomSheetControl, close() now returns false for four different outcomes, and closeAll still cannot say whether an interceptor stopped the cascade. --- API-REVIEW.md | 47 ++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/API-REVIEW.md b/API-REVIEW.md index 3d1cfef..af39cbf 100644 --- a/API-REVIEW.md +++ b/API-REVIEW.md @@ -429,8 +429,49 @@ B1, B2, B4, B6, B7, B8, B9 — done. B3, B5, P1, P5, P7, P10, and the dead code in section 4 — done. **Stage 3 — cleanup (breaking, 2.0):** -P2, P4, P6, P8, P9, W1–W8 — done. +P2, P4, P6, P8, P9, W2, W3, W5, W6, W7, W8 — done. + +### Still open + +- **W1** (three naming conventions for context hooks) — not done. + `useMaybeBottomSheetContext`, `useBottomSheetRefContext` and + `useBottomSheetDefaultIndex` still disagree, and the manager hook still lives + in the provider file rather than the context file. +- **W4** (discriminated union for the `open()` payload) — not done. The mode is + still encoded in the `usePortal` + `keepMounted` + `content` combination, of + which only three of eight are real. +- **P3** is partly done: `useBottomSheetManager().open()` now accepts `params`, + but see the asymmetry below. + +### Introduced by this work + +- `useBottomSheetManager().open()` returns `string | null`, but + `useBottomSheetControl().open()` still returns `void` — the store hands it an + `OpenResult`, which it consumes internally and drops. The same rejection is + visible through one hook and invisible through the other. +- `requestClose` (and therefore every `close()`) now returns `false` for four + distinct outcomes: the sheet was already closing, the interceptor returned + false, the interceptor threw, and there was nothing to close. Fixing B9 made + the value honest about "is it closing" at the cost of conflating why not. +- `closeAllAnimated` still returns `Promise`, so a cascade stopped by an + interceptor is indistinguishable from one that closed everything — even + though the docs say it can stop. + +### Not attempted + +`BottomSheetPortal` reads `getSheetRef(id)` during render — a module-global map +consulted from render, which is not reactive. It works only because +`portalSession` changes in the same store write that creates the ref. Fragile, +but untangling it means reworking how portals learn about refs, which is a +larger change than anything here. + +The native detent cap derives from `getLocationInWindow` / `convert(to: window)`, +which account for transforms — so a sheet scaled by another above it (via +`ScaleWrapper`) may recompute its cap mid-animation and twitch. None of this is covered by a test or verified on a device; the repo still has no -test suite. The changes most worth exercising on hardware are B5 (the three -adapters whose backdrop timing changed) and B7 (the coordinator's close path). +test suite. That is the largest outstanding gap: this work changed the store's +shape, the open/close contract and the animation timing of three adapters, and +nothing guards any of it. The changes most worth exercising on hardware are B5 +(the three adapters whose backdrop timing changed) and B7 (the coordinator's +close path). From 6d57014ea7d7d2112667ca7c0792319b9f9f7be8 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 4 Aug 2026 20:14:10 +0000 Subject: [PATCH 07/13] refactor!: close W1/W4 and the open/close reporting asymmetries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit BREAKING CHANGE: close() and closeAll() resolve to result objects instead of booleans/void; useBottomSheetControl().open() returns boolean; useBottomSheetRefContext is now useMaybeBottomSheetRef. W4 — open() takes a discriminated OpenPayload (`kind: 'inline'` carrying content, or `kind: 'portal'` without) and mount() takes MountPayload. The old shape encoded the mode across usePortal + keepMounted + content, which can express eight combinations of which three are real, and made callers pass `content: null` purely to signal "not inline". The store maps `kind` onto its internal flags in one place, so no caller knows the encoding. W1 — one naming rule for context hooks: `useMaybe*` may return null/undefined and leaves handling it to the caller; plain `use*` either throws or resolves to a documented default. useBottomSheetRefContext became useMaybeBottomSheetRef accordingly, and the manager hooks moved out of the provider file into BottomSheetManager.context.tsx, where the context lives. The three asymmetries the review introduced: - useBottomSheetControl().open() dropped the OpenResult it consumed internally, so a rejection was visible through useBottomSheetManager and invisible here. It now returns boolean — each hook reports rejection in the currency that is useful there, an ID or a yes/no. - close() returned false for four distinct outcomes. It now resolves to a CloseResult carrying a reason: 'blocked', 'interceptor-error' or 'not-closable'. Callers can tell a refusal from nothing-to-close. - closeAll() returned Promise, so a cascade stopped by an interceptor looked exactly like one that closed everything. It now resolves to a CloseAllResult with what closed and which sheet stopped it. While adding that reason, a related bug surfaced: closeAllAnimated treated 'nothing to close' as a refusal and broke out of the loop, stranding every sheet below a sheet that had settled or vanished mid-cascade. Only a real refusal stops it now. --- API-REVIEW.md | 45 ++++++++---------- docs/docs/api/hooks.md | 37 ++++++++++++--- docs/docs/api/types.md | 2 +- src/BottomSheetHost.tsx | 2 +- src/BottomSheetManager.context.tsx | 24 ++++++++++ src/BottomSheetManager.provider.tsx | 29 +---------- src/BottomSheetPersistent.tsx | 4 +- src/BottomSheetRef.context.ts | 7 +-- src/bottomSheetCoordinator.ts | 47 +++++++++++------- src/index.tsx | 3 ++ src/store/store.ts | 37 +++++++++++---- src/store/types.ts | 74 +++++++++++++++++++++++++++-- src/useAdapterRef.ts | 4 +- src/useBottomSheetContext.ts | 7 +-- src/useBottomSheetControl.ts | 34 +++++++++---- src/useBottomSheetManager.tsx | 3 +- src/useScaleAnimation.ts | 2 +- src/useSheetRenderData.ts | 2 +- 18 files changed, 252 insertions(+), 111 deletions(-) diff --git a/API-REVIEW.md b/API-REVIEW.md index af39cbf..70e2bbc 100644 --- a/API-REVIEW.md +++ b/API-REVIEW.md @@ -431,31 +431,26 @@ B3, B5, P1, P5, P7, P10, and the dead code in section 4 — done. **Stage 3 — cleanup (breaking, 2.0):** P2, P4, P6, P8, P9, W2, W3, W5, W6, W7, W8 — done. -### Still open - -- **W1** (three naming conventions for context hooks) — not done. - `useMaybeBottomSheetContext`, `useBottomSheetRefContext` and - `useBottomSheetDefaultIndex` still disagree, and the manager hook still lives - in the provider file rather than the context file. -- **W4** (discriminated union for the `open()` payload) — not done. The mode is - still encoded in the `usePortal` + `keepMounted` + `content` combination, of - which only three of eight are real. -- **P3** is partly done: `useBottomSheetManager().open()` now accepts `params`, - but see the asymmetry below. - -### Introduced by this work - -- `useBottomSheetManager().open()` returns `string | null`, but - `useBottomSheetControl().open()` still returns `void` — the store hands it an - `OpenResult`, which it consumes internally and drops. The same rejection is - visible through one hook and invisible through the other. -- `requestClose` (and therefore every `close()`) now returns `false` for four - distinct outcomes: the sheet was already closing, the interceptor returned - false, the interceptor threw, and there was nothing to close. Fixing B9 made - the value honest about "is it closing" at the cost of conflating why not. -- `closeAllAnimated` still returns `Promise`, so a cascade stopped by an - interceptor is indistinguishable from one that closed everything — even - though the docs say it can stop. +### Closed in a follow-up + +- **W1** — one rule now: a `useMaybe*` hook may return `null`/`undefined` and + leaves handling that to the caller; a plain `use*` hook either throws or + resolves to a documented default. `useBottomSheetRefContext` became + `useMaybeBottomSheetRef`, and the manager hooks moved from the provider file + to `BottomSheetManager.context.tsx` where the context itself lives. +- **W4** — `open()` takes a discriminated `OpenPayload` (`kind: 'inline'` with + content, or `kind: 'portal'` without), and `mount()` takes `MountPayload`. + `content: null` as a "not inline" signal is gone, and the store maps `kind` + onto its internal flags in one place. +- `useBottomSheetControl().open()` now returns `boolean`. It and + `useBottomSheetManager().open()` both report rejection, each in the currency + that is useful there — an ID or a yes/no. +- `close()` returns a `CloseResult` carrying a reason (`'blocked'`, + `'interceptor-error'`, `'not-closable'`) instead of a boolean that meant four + different things. +- `closeAll()` returns a `CloseAllResult` naming what closed and which sheet + stopped the cascade. It also no longer treats a sheet that had nothing to + close as a refusal — that used to strand every sheet below it. ### Not attempted diff --git a/docs/docs/api/hooks.md b/docs/docs/api/hooks.md index 30081cd..fa7bce2 100644 --- a/docs/docs/api/hooks.md +++ b/docs/docs/api/hooks.md @@ -29,8 +29,8 @@ const { open, close, closeAll, destroyAll } = useBottomSheetManager(); | Property | Type | Description | |----------|------|-------------| | `open` | `(content, options?) => string \| null` | Opens a bottom sheet and returns its ID, or `null` if the store declined | -| `close` | `(id: string) => Promise` | Closes a specific sheet by ID. Resolves `false` if an interceptor blocked it | -| `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | +| `close` | `(id: string) => Promise` | Closes a specific sheet by ID | +| `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | | `destroyAll` | `() => void` | Removes all sheets immediately — no animation, **bypasses `onBeforeClose`** | ### closeAll Options @@ -86,10 +86,33 @@ if (id === null) { |---|---|---| | Animation | staggered cascade | none | | `onBeforeClose` | respected | **bypassed** | -| Returns | `Promise` | `void` | +| Returns | `Promise` | `void` | `destroyAll()` is a teardown primitive — it drops every sheet in the group from the store immediately, without asking an interceptor that may be guarding unsaved work. Use `closeAll()` for anything user-facing. +### Close results + +Every `close()` resolves to a `CloseResult`, and `closeAll()` to a `CloseAllResult`. Both carry more than a boolean, because "the user declined" and "there was nothing to close" are different answers: + +```tsx +const result = await close(id); +if (!result.closed) { + switch (result.reason) { + case 'blocked': // an onBeforeClose interceptor said no + case 'interceptor-error': // the interceptor threw; cancelled for safety + case 'not-closable': // already closing, hidden, or unknown sheet + } +} + +const cascade = await closeAll(); +if (!cascade.closedAll) { + // cascade.stoppedAt — the sheet whose interceptor stopped it. + // cascade.closed — the ones that did close, topmost first. +} +``` + +A sheet with nothing to close no longer stops a cascade — only a refusal does. + --- ## useBottomSheetContext @@ -125,7 +148,7 @@ console.log(params.userId); // type-safe: string | `id` | `string` | Current sheet's ID | | `params` | `BottomSheetPortalParams` or `unknown` | Type-safe params when generic provided | | `preventDismiss` | `boolean` | Whether dismissal is currently blocked for this sheet (set via `useOnBeforeClose`). Useful for UI that should reflect it — e.g. hiding a grab handle. | -| `close` | `() => Promise` | Closes this sheet (respects `useOnBeforeClose`). Resolves `true` once it is closing, `false` if an interceptor blocked it or there was nothing to close. | +| `close` | `() => Promise` | Closes this sheet (respects `useOnBeforeClose`). See [Close results](#close-results). | | `forceClose` | `() => void` | Closes this sheet immediately, bypassing any `useOnBeforeClose` interceptor | --- @@ -152,9 +175,9 @@ const { open, close, closeAll, updateParams, resetParams } = useBottomSheetContr | Property | Type | Description | |----------|------|-------------| -| `open` | `(options?) => void` | Opens the sheet | -| `close` | `() => Promise` | Closes the sheet (respects `useOnBeforeClose`). Resolves `false` if an interceptor blocked it | -| `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | +| `open` | `(options?) => boolean` | Opens the sheet. `false` if the store declined | +| `close` | `() => Promise` | Closes the sheet (respects `useOnBeforeClose`) | +| `closeAll` | `(options?) => Promise` | Closes all sheets with cascading animation | | `updateParams` | `(params) => void` | Updates the sheet's params | | `resetParams` | `() => void` | Resets params to `undefined` | diff --git a/docs/docs/api/types.md b/docs/docs/api/types.md index 125d0d3..24354cb 100644 --- a/docs/docs/api/types.md +++ b/docs/docs/api/types.md @@ -253,7 +253,7 @@ interface UseBottomSheetContextReturn { id: string; params: TParams; preventDismiss: boolean; - close: () => Promise; + close: () => Promise; forceClose: () => void; } ``` diff --git a/src/BottomSheetHost.tsx b/src/BottomSheetHost.tsx index a8e83b9..57baced 100644 --- a/src/BottomSheetHost.tsx +++ b/src/BottomSheetHost.tsx @@ -2,7 +2,7 @@ import { useEffect } from 'react'; import { useClearGroup } from './store'; import { initBottomSheetCoordinator } from './bottomSheetCoordinator'; -import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; +import { useBottomSheetManagerContext } from './BottomSheetManager.context'; import { QueueItem } from './QueueItem'; import { useSheetRenderData } from './useSheetRenderData'; diff --git a/src/BottomSheetManager.context.tsx b/src/BottomSheetManager.context.tsx index 010eade..f3d38b2 100644 --- a/src/BottomSheetManager.context.tsx +++ b/src/BottomSheetManager.context.tsx @@ -8,3 +8,27 @@ export interface BottomSheetManagerContextValue { export const BottomSheetManagerContext = React.createContext(null); + +/** + * The enclosing manager's context. Throws outside a provider. + * + * Naming convention across the context files: a `useMaybe*` hook can return + * `null`/`undefined` and leaves handling that to the caller; a plain `use*` + * hook either throws or resolves to a documented default. + */ +export const useBottomSheetManagerContext = + (): BottomSheetManagerContextValue => { + const context = React.useContext(BottomSheetManagerContext); + + if (!context) { + throw new Error( + 'useBottomSheetManagerContext must be used within a BottomSheetManagerProvider' + ); + } + return context; + }; + +/** As {@link useBottomSheetManagerContext}, but `null` outside a provider. */ +export const useMaybeBottomSheetManagerContext = + (): BottomSheetManagerContextValue | null => + React.useContext(BottomSheetManagerContext); diff --git a/src/BottomSheetManager.provider.tsx b/src/BottomSheetManager.provider.tsx index 5881c56..deeffc0 100644 --- a/src/BottomSheetManager.provider.tsx +++ b/src/BottomSheetManager.provider.tsx @@ -1,10 +1,7 @@ -import React, { type PropsWithChildren } from 'react'; +import { type PropsWithChildren } from 'react'; import { PortalProvider } from 'react-native-teleport'; -import { - BottomSheetManagerContext, - type BottomSheetManagerContextValue, -} from './BottomSheetManager.context'; +import { BottomSheetManagerContext } from './BottomSheetManager.context'; import type { ScaleConfig } from './useScaleAnimation'; interface ProviderProps extends PropsWithChildren { @@ -27,25 +24,3 @@ export function BottomSheetManagerProvider({ ); } - -export const useBottomSheetManagerContext = - (): BottomSheetManagerContextValue => { - const context = React.useContext(BottomSheetManagerContext); - - if (!context) { - throw new Error( - 'useBottomSheetManagerContext must be used within a BottomSheetManagerProvider' - ); - } - return context; - }; - -export const useMaybeBottomSheetManagerContext = - (): BottomSheetManagerContextValue | null => { - const context = React.useContext(BottomSheetManagerContext); - - if (!context) { - return null; - } - return context; - }; diff --git a/src/BottomSheetPersistent.tsx b/src/BottomSheetPersistent.tsx index 8e9996f..249bff8 100644 --- a/src/BottomSheetPersistent.tsx +++ b/src/BottomSheetPersistent.tsx @@ -11,7 +11,7 @@ import { useUnmount, } from './store'; import { BottomSheetDefaultIndexContext } from './BottomSheetDefaultIndex.context'; -import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; +import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.context'; import { BottomSheetRefContext } from './BottomSheetRef.context'; import type { BottomSheetPortalId } from './portal.types'; import { setSheetRef } from './refsMap'; @@ -35,7 +35,7 @@ export function BottomSheetPersistent({ const groupId = bottomSheetManagerContext?.groupId || 'default'; const mountSheet = useStableCallback(() => { - mount({ id, groupId, content: null, usePortal: true, keepMounted: true }); + mount({ id, groupId }); }); useEffect(() => { diff --git a/src/BottomSheetRef.context.ts b/src/BottomSheetRef.context.ts index 4546e9b..69a3e3c 100644 --- a/src/BottomSheetRef.context.ts +++ b/src/BottomSheetRef.context.ts @@ -2,11 +2,12 @@ import { createContext, useContext } from 'react'; import type { SheetRef } from './adapter.types'; /** - * Context for passing sheet ref from BottomSheetPersistent/BottomSheetPortal - * to the adapter. This allows automatic ref binding without user intervention. + * Carries the sheet ref from `BottomSheetPersistent` / `BottomSheetPortal` down + * to the adapter, so ref binding needs no user intervention. */ export const BottomSheetRefContext = createContext(null); -export function useBottomSheetRefContext(): SheetRef | null { +/** The enclosing sheet's ref, or `null` outside a portal/persistent sheet. */ +export function useMaybeBottomSheetRef(): SheetRef | null { return useContext(BottomSheetRefContext); } diff --git a/src/bottomSheetCoordinator.ts b/src/bottomSheetCoordinator.ts index e42c502..520c70a 100644 --- a/src/bottomSheetCoordinator.ts +++ b/src/bottomSheetCoordinator.ts @@ -1,5 +1,6 @@ import type { SheetAdapterEvents } from './adapter.types'; import { useBottomSheetStore } from './store'; +import type { CloseAllResult, CloseResult } from './store'; import { getOnBeforeClose } from './onBeforeCloseRegistry'; import { getSheetRef } from './refsMap'; @@ -94,18 +95,18 @@ export function initBottomSheetCoordinator(groupId: string) { * If an onBeforeClose callback is registered for the sheet and it returns * `false` (or resolves to `false`), the close is cancelled. * - * @returns `true` if the sheet is now closing, `false` if the interceptor - * blocked it — or if there was nothing to close (the sheet is already closing, - * hidden, or does not exist). + * @returns A {@link CloseResult}. `closed: false` carries a reason, because + * "the interceptor declined" and "there was nothing to close" are different + * answers that callers routinely need to tell apart. */ -export async function requestClose(sheetId: string): Promise { +export async function requestClose(sheetId: string): Promise { const state = useBottomSheetStore.getState(); const currentStatus = state.sheetsById[sheetId]?.status; // Don't run interceptor if sheet is already closing // This prevents duplicate interceptor calls during close animations if (currentStatus === 'closing') { - return false; + return { closed: false, reason: 'not-closable' }; } const interceptor = getOnBeforeClose(sheetId); @@ -134,7 +135,7 @@ export async function requestClose(sheetId: string): Promise { }); if (!allowed) { - return false; + return { closed: false, reason: 'blocked' }; } } catch (error) { // If the interceptor throws, cancel the close for safety @@ -145,19 +146,19 @@ export async function requestClose(sheetId: string): Promise { error ); } - return false; + return { closed: false, reason: 'interceptor-error' }; } } if (currentStatus === 'open' || currentStatus === 'opening') { state.startClosing(sheetId); - return true; + return { closed: true }; } // Nothing to close: hidden, already gone, or a status that cannot transition - // to closing. The interceptor did not block, but the sheet is not closing - // either — say so rather than reporting a close that never happened. - return false; + // to closing. No interceptor had an opinion — which is why this is its own + // reason rather than being folded into `blocked`. + return { closed: false, reason: 'not-closable' }; } /** @@ -176,18 +177,21 @@ const DEFAULT_STAGGER_MS = 100; * * @param groupId - The manager group to close sheets in. * @param options.stagger - Delay in ms between each close (default: 100). - * @returns A promise that resolves when the cascade finishes (or is stopped). + * @returns A {@link CloseAllResult} naming what closed and, if the cascade was + * stopped, which sheet stopped it. Without this a blocked cascade is + * indistinguishable from one that closed everything. */ export async function closeAllAnimated( groupId: string, options?: { stagger?: number } -): Promise { +): Promise { const stagger = options?.stagger ?? DEFAULT_STAGGER_MS; const state = useBottomSheetStore.getState(); // Close from top to bottom (reverse order) const reversed = [...(state.stackOrderByGroup[groupId] ?? [])].reverse(); + const closed: string[] = []; for (const [index, sheetId] of reversed.entries()) { const currentState = useBottomSheetStore.getState(); @@ -198,17 +202,26 @@ export async function closeAllAnimated( continue; } - const closed = await requestClose(sheetId); + const result = await requestClose(sheetId); - if (!closed) { - // Interceptor blocked — stop the cascade - break; + if (!result.closed) { + if (result.reason === 'not-closable') { + // Nothing to close here (it settled or vanished mid-cascade); that is + // not a refusal, so keep going rather than stranding the sheets below. + continue; + } + // An interceptor declined — stop and report where. + return { closedAll: false, closed, stoppedAt: sheetId }; } + closed.push(sheetId); + if (stagger > 0 && index < reversed.length - 1) { await new Promise((resolve) => setTimeout(resolve, stagger)); } } + + return { closedAll: true, closed }; } /** diff --git a/src/index.tsx b/src/index.tsx index 77e933a..b83b4ae 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -64,6 +64,9 @@ export type { OpenMode, OpenResult, OpenRejectionReason, + CloseResult, + CloseRejectionReason, + CloseAllResult, PublicBottomSheetState as BottomSheetState, } from './store'; export type { diff --git a/src/store/store.ts b/src/store/store.ts index a578c11..aa6ce52 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -17,10 +17,24 @@ import { getNextPortalSession } from '../portalSessionRegistry'; import type { BottomSheetState, BottomSheetStore, + OpenPayload, OpenRejectionReason, OpenResult, } from './types'; +/** + * Translates the public `kind` discriminant into the flags the store records. + * + * `kind` is what callers reason about; `usePortal` is what the renderer checks. + * Keeping the mapping in one place means no caller has to know the encoding. + */ +function toStoreFields(sheet: OpenPayload) { + const { kind, ...rest } = sheet; + return kind === 'inline' + ? { ...rest, usePortal: false } + : { ...rest, usePortal: true, content: undefined }; +} + function warnRejectedOpen(id: string, reason: OpenRejectionReason) { if (!__DEV__) return; @@ -68,8 +82,10 @@ export const useBottomSheetStore = create( mode ); + const fields = toStoreFields(sheet); + const shouldGetNewPortalSession = - sheet.usePortal && (!existingSheet || !existingSheet.keepMounted); + fields.usePortal && (!existingSheet || !existingSheet.keepMounted); const nextPortalSession = shouldGetNewPortalSession ? getNextPortalSession(sheet.id) : undefined; @@ -88,7 +104,7 @@ export const useBottomSheetStore = create( ? existingSheet.portalSession : (nextPortalSession ?? existingSheet.portalSession), } - : { ...sheet, status: 'opening', portalSession: nextPortalSession }; + : { ...fields, status: 'opening', portalSession: nextPortalSession }; return { sheetsById: { ...updatedSheetsById, [sheet.id]: newSheet }, @@ -227,16 +243,19 @@ export const useBottomSheetStore = create( ensureAnimatedIndex(sheet.id); - // For portal-based persistent sheets, set initial portalSession - // This session will be reused across open/close cycles - const portalSession = sheet.usePortal - ? getNextPortalSession(sheet.id) - : undefined; - + // A persistent sheet is portal-based by definition — it stays mounted + // where it was declared and teleports in. The session is allocated once + // and reused across every open/close cycle. return { sheetsById: { ...state.sheetsById, - [sheet.id]: { ...sheet, status: 'hidden', portalSession }, + [sheet.id]: { + ...sheet, + status: 'hidden', + usePortal: true, + keepMounted: true, + portalSession: getNextPortalSession(sheet.id), + }, }, }; }), diff --git a/src/store/types.ts b/src/store/types.ts index aef8959..0277cfc 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -46,7 +46,40 @@ export type PublicBottomSheetState = Pick< 'id' | 'groupId' | 'status' | 'params' | 'scaleBackground' | 'keepMounted' >; -export type TriggerState = Omit; +/** Fields every open payload carries, regardless of how the sheet renders. */ +interface OpenPayloadBase { + id: string; + groupId: string; + scaleBackground?: boolean; + backdrop?: boolean; + params?: Record; +} + +/** + * What `open()` accepts, as a discriminated union of the modes the library + * actually documents. + * + * The alternative — a single bag with optional `content` / `usePortal` / + * `keepMounted` — can express eight combinations of which only three are real, + * and forces callers to pass `content: null` just to signal "not inline". + * + * `persistent` is not a variant here: a persistent sheet is registered with + * `mount()` and re-opened as `portal`, keeping the `keepMounted` flag its + * store record already carries. + */ +export type OpenPayload = + | (OpenPayloadBase & { + /** Content supplied at call time; unmounted on close. */ + kind: 'inline'; + content: ReactNode; + }) + | (OpenPayloadBase & { + /** Content declared elsewhere and teleported in, preserving context. */ + kind: 'portal'; + }); + +/** What `mount()` accepts — a persistent sheet, pre-registered as hidden. */ +export type MountPayload = OpenPayloadBase; /** Why an `open()` call did not put the sheet on the stack. */ export type OpenRejectionReason = @@ -63,6 +96,41 @@ export type OpenResult = | { opened: true; id: string } | { opened: false; id: string; reason: OpenRejectionReason }; +/** Why a close did not happen. */ +export type CloseRejectionReason = + /** An `onBeforeClose` interceptor declined. */ + | 'blocked' + /** The interceptor threw; the close is cancelled for safety. */ + | 'interceptor-error' + /** + * There was nothing to close: the sheet is already closing, is hidden, or + * the store has no record of it. Distinct from `blocked` — no interceptor + * had an opinion. + */ + | 'not-closable'; + +/** + * Outcome of a close. Carries a reason rather than a bare boolean, because + * "the user declined" and "there was nothing to close" are different answers + * and callers routinely need to tell them apart. + */ +export type CloseResult = + | { closed: true } + | { closed: false; reason: CloseRejectionReason }; + +/** Outcome of a cascading close. */ +export interface CloseAllResult { + /** Whether every sheet in the group closed. */ + closedAll: boolean; + /** IDs that closed, topmost first. */ + closed: string[]; + /** + * The sheet whose interceptor stopped the cascade, if one did. Sheets below + * it were left open. + */ + stoppedAt?: string; +} + export interface BottomSheetStoreState { sheetsById: Record; /** @@ -76,7 +144,7 @@ export interface BottomSheetStoreState { } export interface BottomSheetStoreActions { - open(sheet: TriggerState, mode?: OpenMode): OpenResult; + open(sheet: OpenPayload, mode?: OpenMode): OpenResult; markOpen(id: string): void; startClosing(id: string): void; finishClosing(id: string): void; @@ -85,7 +153,7 @@ export interface BottomSheetStoreActions { setBackdrop(id: string, backdrop: boolean): void; clearGroup(groupId: string): void; clearAll(): void; - mount(sheet: TriggerState): void; + mount(sheet: MountPayload): void; unmount(id: string): void; } diff --git a/src/useAdapterRef.ts b/src/useAdapterRef.ts index 75cd0e6..eac2368 100644 --- a/src/useAdapterRef.ts +++ b/src/useAdapterRef.ts @@ -1,7 +1,7 @@ import type { ForwardedRef } from 'react'; import type { SheetAdapterRef, SheetRef } from './adapter.types'; -import { useBottomSheetRefContext } from './BottomSheetRef.context'; +import { useMaybeBottomSheetRef } from './BottomSheetRef.context'; /** * Returns the correct ref for a custom adapter. @@ -23,6 +23,6 @@ import { useBottomSheetRefContext } from './BottomSheetRef.context'; export function useAdapterRef( forwardedRef: ForwardedRef ): SheetRef | ForwardedRef { - const contextRef = useBottomSheetRefContext(); + const contextRef = useMaybeBottomSheetRef(); return contextRef ?? forwardedRef; } diff --git a/src/useBottomSheetContext.ts b/src/useBottomSheetContext.ts index 9e5aee2..1653245 100644 --- a/src/useBottomSheetContext.ts +++ b/src/useBottomSheetContext.ts @@ -4,6 +4,7 @@ import { useSheetPreventDismiss, useStartClosing, } from './store'; +import type { CloseResult } from './store'; import { requestClose } from './bottomSheetCoordinator'; import type { BottomSheetPortalId, @@ -29,10 +30,10 @@ export interface UseBottomSheetContextReturn { /** * Closes the sheet. * - * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` - * interceptor blocked it or there was nothing to close. + * @returns A `CloseResult` — `{ closed: true }`, or `{ closed: false, reason }` + * naming why not (`'blocked'`, `'interceptor-error'`, `'not-closable'`). */ - close: () => Promise; + close: () => Promise; /** * Close the sheet, bypassing any onBeforeClose interceptor. * Useful for force-closing from within onBeforeClose confirmation flows. diff --git a/src/useBottomSheetControl.ts b/src/useBottomSheetControl.ts index 93ca663..e27c134 100644 --- a/src/useBottomSheetControl.ts +++ b/src/useBottomSheetControl.ts @@ -2,7 +2,8 @@ import React from 'react'; import type { SheetAdapterRef } from './adapter.types'; import { useOpen, useUpdateParams, type OpenMode } from './store'; -import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; +import type { CloseAllResult, CloseResult } from './store'; +import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.context'; import { closeAllAnimated, requestClose } from './bottomSheetCoordinator'; import type { BottomSheetPortalId, @@ -29,19 +30,34 @@ type OpenOptions = Omit< type OpenFunction = HasParams extends true - ? (options: OpenOptions) => void - : (options?: OpenOptions) => void; + ? (options: OpenOptions) => boolean + : (options?: OpenOptions) => boolean; export interface UseBottomSheetControlReturn { + /** + * Opens the sheet. + * + * @returns `false` when the store declined — the sheet is already on the + * stack, or another sheet in the group is still animating open. A `__DEV__` + * warning explains which. (`useBottomSheetManager().open()` reports the same + * rejection as `null` instead of an ID, since there the ID is the useful + * half of the answer; here you already know it.) + */ open: OpenFunction; /** * Closes the sheet. * - * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` - * interceptor blocked it or there was nothing to close. + * @returns A `CloseResult` — `{ closed: true }`, or `{ closed: false, reason }` + * naming why not (`'blocked'`, `'interceptor-error'`, `'not-closable'`). */ - close: () => Promise; - closeAll: (options?: CloseAllOptions) => Promise; + close: () => Promise; + /** + * Closes every sheet in the group, topmost first. + * + * @returns A `CloseAllResult` — what closed, and which sheet stopped the + * cascade if an interceptor did. + */ + closeAll: (options?: CloseAllOptions) => Promise; updateParams: (params: BottomSheetPortalParams) => void; resetParams: () => void; } @@ -59,9 +75,9 @@ export function useBottomSheetControl( const result = storeOpen( { + kind: 'portal', id, groupId, - usePortal: true, scaleBackground: options?.scaleBackground, backdrop: options?.backdrop, params: options?.params as Record, @@ -75,6 +91,8 @@ export function useBottomSheetControl( if (result.opened && !getSheetRef(id)) { setSheetRef(id, React.createRef()); } + + return result.opened; }; const close = () => requestClose(id); diff --git a/src/useBottomSheetManager.tsx b/src/useBottomSheetManager.tsx index d2e840e..8b7e519 100644 --- a/src/useBottomSheetManager.tsx +++ b/src/useBottomSheetManager.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { useOpen, useClearGroup, type OpenMode } from './store'; -import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.provider'; +import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.context'; import type { SheetAdapterRef } from './adapter.types'; import { closeAllAnimated, requestClose } from './bottomSheetCoordinator'; import { setSheetRef } from './refsMap'; @@ -47,6 +47,7 @@ export const useBottomSheetManager = () => { const result = storeOpen( { + kind: 'inline', id, groupId, content: contentWithRef, diff --git a/src/useScaleAnimation.ts b/src/useScaleAnimation.ts index 9258dd4..73ce412 100644 --- a/src/useScaleAnimation.ts +++ b/src/useScaleAnimation.ts @@ -8,7 +8,7 @@ import { type WithTimingConfig, } from 'react-native-reanimated'; import { useBottomSheetStore } from './store'; -import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; +import { useBottomSheetManagerContext } from './BottomSheetManager.context'; export type ScaleAnimationConfig = | { type: 'timing'; config?: WithTimingConfig } diff --git a/src/useSheetRenderData.ts b/src/useSheetRenderData.ts index 40eebc8..3613966 100644 --- a/src/useSheetRenderData.ts +++ b/src/useSheetRenderData.ts @@ -1,5 +1,5 @@ import { useBottomSheetStore, type BottomSheetState } from './store'; -import { useBottomSheetManagerContext } from './BottomSheetManager.provider'; +import { useBottomSheetManagerContext } from './BottomSheetManager.context'; export interface SheetRenderItem { id: string; From 8631dcc64032d2c095d65e14121eec6f867cf3b0 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 06:24:42 +0000 Subject: [PATCH 08/13] fix: address review findings across store, coordinator and adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A four-way review of this PR and the swmansion bump found bugs that the original work missed. Each behavioural fix below was reproduced before and verified after. Store ----- open() could push a duplicate ID onto a group stack: switch mode leaves the previous top hidden but still on the stack, and a persistent sheet in that state satisfies the activatable guard, so re-opening it appended a second entry (['p','q','p']) — duplicate React keys and a QueueItem at the wrong z-index. The push now dedupes. Re-opening a persistent sheet from a different group desynced its record from the stack it landed in: the existing-sheet branch never applied the payload's groupId while the stack write did, leaving the sheet unremovable from the second group forever. Rejected now, with a new 'group-mismatch' reason. startClosing restored the sheet below even when the closing sheet was not topmost, so a switched-away sheet animated back in underneath the current top and — because the busy guard keys on 'opening' — blocked the group. Gated on being the group top, matching finishClosing. unmount removed a sheet without restoring the one below, unlike finishClosing. BottomSheetPersistent calls unmount on every component unmount, so navigating away left a hidden sheet rendered as active. Both paths now share detachFromGroup(). Coordinator ----------- An interceptor returning literal false hung requestClose() forever: the truthiness gate dropped it, and closeAllAnimated stalled awaiting a promise that never settled. Discriminated on type now. (This shipped broken in this PR; the tests branch had already fixed it independently.) requestClose branched on a status captured before awaiting the interceptor, so a sheet removed during a long Alert.alert still reported success. Re-read after the await. A sheet whose ref never arrived wedged its whole group unrecoverably — driveSheetRef gave up but left it 'opening', and the busy guard then rejected every later open. It now forces the sheet closed. Adapters -------- detached rendered square bottom corners in every case except detached+fullHeight. The native surface is sized to the whole container and translated down, so its rounded bottom edge sat below the frame and the rectangular clip cut it off. The clipping frame now carries the bottom radii. Removed scrollableNegotiation: 0.16.2 does not know the prop and BottomSheet destructures known props rather than spreading a rest object, so it was provably inert on every version the peer range allows. It returns for free, correctly typed by the library, when the peer moves to 0.17. ActionsSheetAdapter painted its own 0.3 overlay over the manager's backdrop (the library renders one regardless of isModal), and its preventDismiss wiring disabled back and backdrop entirely, so an interceptor never ran and the sheet became undismissable. BottomSheetHost subscribed in an effect without reconciling, so any status transition from an earlier sibling's mount effect was lost and the sheet stayed 'opening' forever. Also: raised the reanimated peer floor to >=4.0.0 (adapters call SharedValue.set and the core-entry adapter imports scheduleOnRN from worklets, so >=3.0.0 installed cleanly and crashed at runtime), deleted the unused src/adapters/index.ts barrel that would have pulled every optional peer into the main entry, dropped memo() from QueueItem per the project's own React Compiler rule, removed dead code, and trimmed comments that restated their code. --- docs/docs/built-in-adapters/swmansion.md | 13 +- .../src/sheets/ThirdPartyAdapterSheets.tsx | 5 +- package.json | 2 +- src/BottomSheetHost.tsx | 75 ++++++++- src/QueueItem.tsx | 17 +- .../actions-sheet/ActionsSheetAdapter.tsx | 21 ++- src/adapters/index.ts | 17 -- .../ReactNativeModalAdapter.tsx | 4 - .../swmansion/SwmansionSheetAdapter.tsx | 41 +---- src/adapters/swmansion/index.ts | 2 - src/animatedRegistry.ts | 18 +-- src/bottomSheetCoordinator.ts | 47 +++--- src/refsMap.ts | 8 + src/store/helpers.ts | 40 ++++- src/store/store.ts | 145 ++++++++---------- src/store/types.ts | 14 +- src/testing.ts | 17 +- src/useBackHandler.ts | 10 +- src/useBottomSheetManager.tsx | 7 +- src/useOnBeforeClose.ts | 29 +--- src/useScaleAnimation.ts | 13 +- src/useSheetRenderData.ts | 15 +- src/useStableCallback.ts | 2 + 23 files changed, 309 insertions(+), 253 deletions(-) delete mode 100644 src/adapters/index.ts diff --git a/docs/docs/built-in-adapters/swmansion.md b/docs/docs/built-in-adapters/swmansion.md index 26a4072..4b33024 100644 --- a/docs/docs/built-in-adapters/swmansion.md +++ b/docs/docs/built-in-adapters/swmansion.md @@ -58,15 +58,13 @@ The detent at index `0` must resolve to `0` (collapsed) so the manager can close ## Props -Accepts the full prop surface of [`@swmansion/react-native-bottom-sheet`](https://github.com/software-mansion-labs/react-native-bottom-sheet)'s `BottomSheet` (`detents`, `style`, `extendUnderStatusBar`, `animateContentHeight`, `onIndexChange`, `onSettle`, `onPositionChange`), **except** the props the manager owns: +Accepts the full prop surface of [`@swmansion/react-native-bottom-sheet`](https://github.com/software-mansion-labs/react-native-bottom-sheet)'s `BottomSheet` (`detents`, `style`, `surface`, `extendUnderStatusBar`, `animateContentHeight`, `disableScrollableNegotiation`, `onIndexChange`, `onSettle`), **except** the props the manager owns: - `index` — the adapter is the source of truth. Use `expandedIndex` (a prop added by the adapter, defaults to the last detent) to choose which detent the sheet opens to. - `animateIn` — the manager controls the open animation, so it is forced on. - `onPositionChange` / `wrapNativeView` — consumed by the adapter to drive the backdrop fade on the UI thread. -`scrollableNegotiation` is forwarded too, but the native side only honors it from **0.17**; on 0.16 use the (deprecated) `disableScrollableNegotiation`. - -Your `onIndexChange` / `onSettle` / `onPositionChange` handlers are still invoked after the adapter's own logic. The `programmatic()` helper plus the `Detent`, `DetentValue`, `SwmansionSheetAdapterProps` and `SwmansionHandleConfig` (the `handle` object form) types are exported from the subpath for convenience. +Your `onIndexChange` / `onSettle` handlers are still invoked after the adapter's own logic. The `programmatic()` helper plus the `Detent`, `DetentValue`, `SwmansionSheetAdapterProps` and `SwmansionHandleConfig` (the `handle` object form) types are exported from the subpath for convenience. :::info `onIndexChange` is wider than the native prop The adapter's `onIndexChange` differs from the native one in two ways: @@ -94,10 +92,11 @@ The native sheet is intentionally minimal. The adapter layers a few **opt-in** c | Prop | Type | Default | What it does | | --- | --- | --- | --- | +| `expandedIndex` | `number` | _last detent_ | Index into `detents` the sheet expands to when opened. Replaces the native `index`, which the adapter owns. The detent at index `0` must still resolve to `0`, since that is what the manager snaps back to when closing. | | `handle` | `boolean \| { color?, width?, height? } \| ReactElement` | `false` | Renders a grab handle as a chrome layer over the `surface` and insets the content to clear it. Pass `true` for the default pill, an object to restyle it, or a React element for full control. Auto-hidden when dismissal is blocked (see [Close interception](/close-interception)) — a non-draggable sheet showing a grab handle would mislead. | | `fullHeight` | `boolean` | `false` | Expands the sheet to the full height available to it. swmansion detents are only `number` / `'content'`, and neither expresses "as tall as you can go" — this passes a detent taller than any screen, which native clamps to the height it actually measured. Stays below the status bar unless you also pass `extendUnderStatusBar`; combined with `detached`, it means the detached frame's height. Ignored when explicit `detents` are passed. | | `detached` | `boolean` | `false` | Floats the sheet free of the screen edges — the *detached* presentation from `@gorhom/bottom-sheet`. All four corners are rounded and the sheet rises from the inset frame. See [Detached sheets](#detached-sheets). | -| `bottomInset` | `number` | _safe-area bottom_ | Gap below the sheet. Only meaningful with `detached`. Falls back to `16` where there is no bottom inset. | +| `bottomInset` | `number` | _safe-area bottom, at least `16`_ | Gap below the sheet. Only meaningful with `detached`. | | `horizontalInset` | `number` | `16` | Gap on each side. Only meaningful with `detached`. | | `fillContent` | `boolean` | _auto_ | Stretches the content to fill the sheet (`flex: 1`), so a `flex: 1` scrollable expands and a bottom footer pins to the bottom instead of floating up under the content. Auto and rarely set by hand: `true` for fixed-height sheets (numeric detents or `fullHeight`), `false` for content-sized ones (which must size to their content). Pass a boolean to override. | | `keyboardBehavior` | `'none' \| 'inset'` | `'none'` | Keyboard avoidance — the native sheet has none. `'inset'` insets the content by the keyboard height (works for both content-sized and fixed-height sheets); `'none'` lets the content handle it. See [Keyboard avoidance](#keyboard-avoidance) for when to use which. Reads the keyboard height from `react-native-keyboard-controller`. | @@ -195,7 +194,7 @@ Pick exactly one. Combining `'inset'` with a keyboard-aware scrollable lifts the Detaching works by giving the sheet a **smaller canvas**: the adapter wraps it in a frame inset by those values, and the native host fills that frame. The detent cap is measured from it, so `'content'` and `fullHeight` resolve against the detached height — no arithmetic on your side. All four corners are rounded (an anchored sheet keeps its bottom two square), and the content is clipped to match. -The frame also clips, and that part is load-bearing rather than cosmetic: the native sheet container is a full-canvas view translated down to the current position, and it is explicitly *not* clipped to its host, so its surface hangs below by whatever the sheet has not expanded yet. Unclipped, that surface would paint straight over the bottom gap and the sheet would not read as detached at all. +The frame also clips, and that part is load-bearing rather than cosmetic: the native sheet container is a full-canvas view translated down to the current position, and it is explicitly *not* clipped to its host, so its surface hangs below by whatever the sheet has not expanded yet. Unclipped, that surface would paint straight over the bottom gap and the sheet would not read as detached at all. The same overhang is why the frame — not the surface — carries the bottom corner radii: the surface's own bottom edge sits outside the frame entirely. :::note Shadows on a custom `surface` Because the frame clips, a shadow cast by a custom `surface` is clipped with it. The native sheet itself draws no shadow, so the default surface is unaffected. @@ -213,7 +212,7 @@ swmansion's `scrimColor` / `scrimOpacities` only apply to **modal** sheets. The ## Android back button -This adapter registers a hardware-back handler automatically (via the internal `useBackHandler`): pressing Android back dismisses the top, fully-open sheet — the same contract the other adapters honor. You don't need to wire anything up yourself. +This adapter registers a hardware-back handler automatically (via `useBackHandler`): pressing Android back dismisses the top, fully-open sheet — the same contract the other adapters honor. You don't need to wire anything up yourself. ## When to Use diff --git a/example/src/sheets/ThirdPartyAdapterSheets.tsx b/example/src/sheets/ThirdPartyAdapterSheets.tsx index 835e20e..a327f8a 100644 --- a/example/src/sheets/ThirdPartyAdapterSheets.tsx +++ b/example/src/sheets/ThirdPartyAdapterSheets.tsx @@ -458,8 +458,9 @@ export function SwmansionSheetDemoContent() { * Detached (floating) presentation — the sheet is lifted off the screen edges * instead of anchored to the bottom, with all four corners rounded. * - * The insets default to 16pt horizontally and the bottom safe-area inset, so a - * bare `detached` already clears the home indicator; both are overridable. + * The insets default to 16pt horizontally and the bottom safe-area inset (at + * least 16pt), so a bare `detached` already clears the home indicator; both are + * overridable. */ const DetachedSwmansionSheet = ({ ref }: { ref?: React.Ref }) => { const { close } = useBottomSheetContext(); diff --git a/package.json b/package.json index a9a68a9..2159ab5 100644 --- a/package.json +++ b/package.json @@ -116,7 +116,7 @@ "react-native-gesture-handler": ">=2.0.0", "react-native-keyboard-controller": ">=1.12.0", "react-native-modal": ">=11.0.0", - "react-native-reanimated": ">=3.0.0", + "react-native-reanimated": ">=4.0.0", "react-native-safe-area-context": ">=5.0.0", "react-native-teleport": ">=0.1.0", "react-native-worklets": ">=0.7.0", diff --git a/src/BottomSheetHost.tsx b/src/BottomSheetHost.tsx index 57baced..2fcdd53 100644 --- a/src/BottomSheetHost.tsx +++ b/src/BottomSheetHost.tsx @@ -1,11 +1,82 @@ import { useEffect } from 'react'; -import { useClearGroup } from './store'; +import { useBottomSheetStore, useClearGroup } from './store'; import { initBottomSheetCoordinator } from './bottomSheetCoordinator'; import { useBottomSheetManagerContext } from './BottomSheetManager.context'; import { QueueItem } from './QueueItem'; +import { getSheetRef } from './refsMap'; import { useSheetRenderData } from './useSheetRenderData'; +/** + * Frames to keep retrying the initial reconcile before giving up — the adapter + * ref appears a frame or two after the sheet enters the store, and a portal + * sheet has to teleport its content into the `PortalHost` first. + */ +const RECONCILE_MAX_FRAMES = 10; + +/** + * Drives sheets that are already mid-transition when the coordinator subscribes. + * + * The subscription does not fire for state that predates it, and this host's + * effect runs *after* the effects of the content rendered beside it — so a sheet + * opened from an app mount effect writes `'opening'` with nobody listening, and + * would sit in that status forever, blocking every later open in the group. + * + * Only the statuses captured at subscribe time are replayed; anything that moves + * afterwards belongs to the subscription and must not be driven twice. + */ +function reconcilePendingTransitions(groupId: string): () => void { + const initialState = useBottomSheetStore.getState(); + + let pending = (initialState.stackOrderByGroup[groupId] ?? []) + .map((id) => ({ id, status: initialState.sheetsById[id]?.status })) + .filter( + ({ status }) => + status === 'opening' || status === 'closing' || status === 'hidden' + ); + + let framesLeft = RECONCILE_MAX_FRAMES; + let cancelled = false; + + const attempt = () => { + if (cancelled) { + return; + } + + const { sheetsById } = useBottomSheetStore.getState(); + + pending = pending.filter(({ id, status }) => { + if (sheetsById[id]?.status !== status) { + return false; + } + + const ref = getSheetRef(id)?.current; + if (!ref) { + return true; + } + + if (status === 'opening') { + ref.expand(); + } else { + ref.close(); + } + return false; + }); + + if (pending.length > 0 && --framesLeft > 0) { + requestAnimationFrame(attempt); + } + }; + + if (pending.length > 0) { + requestAnimationFrame(attempt); + } + + return () => { + cancelled = true; + }; +} + export function BottomSheetHost() { const sheetRenderData = useSheetRenderData(); const clearGroup = useClearGroup(); @@ -13,7 +84,9 @@ export function BottomSheetHost() { useEffect(() => { const unsubscribe = initBottomSheetCoordinator(groupId); + const cancelReconcile = reconcilePendingTransitions(groupId); return () => { + cancelReconcile(); unsubscribe(); }; }, [groupId]); diff --git a/src/QueueItem.tsx b/src/QueueItem.tsx index 1fb351f..cb8312f 100644 --- a/src/QueueItem.tsx +++ b/src/QueueItem.tsx @@ -1,4 +1,4 @@ -import { memo, useEffect, type PropsWithChildren } from 'react'; +import { useEffect, type PropsWithChildren } from 'react'; import { StyleSheet, View } from 'react-native'; import Animated from 'react-native-reanimated'; import { useSafeAreaFrame } from 'react-native-safe-area-context'; @@ -9,7 +9,6 @@ import { BottomSheetContext } from './BottomSheet.context'; import { useSheetBackdrop, useSheetContent, - useSheetKeepMounted, useSheetPortalSession, useSheetUsePortal, } from './store'; @@ -24,14 +23,9 @@ interface QueueItemProps { isActive: boolean; } -export const QueueItem = memo(function QueueItem({ - id, - stackIndex, - isActive, -}: QueueItemProps) { +export function QueueItem({ id, stackIndex, isActive }: QueueItemProps) { const content = useSheetContent(id); const usePortal = useSheetUsePortal(id); - const keepMounted = useSheetKeepMounted(id); const portalSession = useSheetPortalSession(id); const backdrop = useSheetBackdrop(id); @@ -45,8 +39,11 @@ export const QueueItem = memo(function QueueItem({ cleanupAnimatedIndex(id); removeOnBeforeClose(id); }; - }, [id, keepMounted]); + }, [id]); + // High enough that sheets outrank anything the host app stacks — z-index is + // only comparable within a stacking context, and the manager's layer sits + // alongside app content that is free to use its own values. const baseZIndex = 100_000_000; const backdropZIndex = baseZIndex + stackIndex * 2; @@ -81,7 +78,7 @@ export const QueueItem = memo(function QueueItem({ ); -}); +} const ScaleWrapper = ({ id, diff --git a/src/adapters/actions-sheet/ActionsSheetAdapter.tsx b/src/adapters/actions-sheet/ActionsSheetAdapter.tsx index 56b4cc6..8386a14 100644 --- a/src/adapters/actions-sheet/ActionsSheetAdapter.tsx +++ b/src/adapters/actions-sheet/ActionsSheetAdapter.tsx @@ -27,13 +27,21 @@ const ActionSheet = require('react-native-actions-sheet') * props the stack manager owns: * * - `isModal` — forced off; the manager handles the overlay lifecycle. + * - `defaultOverlayOpacity` — forced to 0; the library paints its own overlay + * whenever `backgroundInteractionEnabled` is falsy, which would stack on top + * of the manager's shared `BottomSheetBackdrop` as a double-dark layer. * - `onOpen` / `onClose` / `onBeforeClose` — consumed by the adapter to report * lifecycle back to the manager. */ export interface ActionsSheetAdapterProps extends Omit< ActionSheetProps, - 'isModal' | 'onOpen' | 'onClose' | 'onBeforeClose' | 'children' + | 'isModal' + | 'defaultOverlayOpacity' + | 'onOpen' + | 'onClose' + | 'onBeforeClose' + | 'children' > { children: React.ReactNode; } @@ -96,16 +104,23 @@ export const ActionsSheetAdapter = React.forwardRef< return ( `: the library declares most of ModalProps -// as required and supplies them through `defaultProps`, so at a call site every -// one of them is genuinely optional. const RNModal = require('react-native-modal').default as React.ComponentType< Partial & { children?: React.ReactNode } >; diff --git a/src/adapters/swmansion/SwmansionSheetAdapter.tsx b/src/adapters/swmansion/SwmansionSheetAdapter.tsx index 5065a03..8cab477 100644 --- a/src/adapters/swmansion/SwmansionSheetAdapter.tsx +++ b/src/adapters/swmansion/SwmansionSheetAdapter.tsx @@ -44,24 +44,6 @@ const { BottomSheet, programmatic } = export { programmatic } from '@swmansion/react-native-bottom-sheet'; export type { Detent, DetentValue }; -/** - * How gestures starting inside a nested scrollable are negotiated with the - * sheet. Mirrors the native prop of the same name. - * - * Requires `@swmansion/react-native-bottom-sheet` **>= 0.17**. Declared here so - * the adapter can forward it on newer versions without pinning the peer to a - * prerelease; on 0.16 the native side ignores it. - */ -export type SwmansionScrollableNegotiationMode = 'none' | 'initial' | 'handoff'; - -/** @see {@link SwmansionScrollableNegotiationMode} */ -export type SwmansionScrollableNegotiation = - | SwmansionScrollableNegotiationMode - | Readonly<{ - expand: SwmansionScrollableNegotiationMode; - collapse: SwmansionScrollableNegotiationMode; - }>; - /** * Style overrides for the adapter-rendered grab handle (the default pill). * @@ -96,9 +78,9 @@ export interface SwmansionHandleConfig { * the status-bar overlap, so a scaled ancestor cannot shift the sheet. * * Every other native prop (`detents`, `style`, `surface`, - * `animateContentHeight`) is forwarded. The `onIndexChange` / `onSettle` - * callbacks are wrapped by the adapter and your handlers are still invoked - * afterwards. + * `animateContentHeight`, `disableScrollableNegotiation`) is forwarded. The + * `onIndexChange` / `onSettle` callbacks are wrapped by the adapter and your + * handlers are still invoked afterwards. * * **`onIndexChange`.** Wider than the native prop: the adapter also fires it for * the programmatic open it drives (at animation start), so you get an immediate @@ -202,7 +184,7 @@ export interface SwmansionSheetAdapterProps * Gap between the bottom of the sheet and the bottom of the screen, in px. * * Only meaningful with {@link detached}. Defaults to the bottom safe-area - * inset, or `16` where there is none. + * inset, but at least `16`. */ bottomInset?: number; /** @@ -232,16 +214,6 @@ export interface SwmansionSheetAdapterProps * is off unless you set this to match its radius. */ cornerRadius?: number; - /** - * Controls how gestures that start in nested scrollables interact with the - * sheet. A string applies to both directions; an object configures expansion - * and collapse independently. - * - * Forwarded as-is to the native sheet, which supports it from **0.17**. On - * 0.16 it is ignored — use the (deprecated) `disableScrollableNegotiation` - * there. - */ - scrollableNegotiation?: SwmansionScrollableNegotiation; /** * Called when the sheet's snap index changes. * @@ -290,10 +262,7 @@ function resolveDetentValue(detent: Detent): DetentValue { /** * Whether the detent at `index` is the collapsed one. * - * The manager treats "settled on a zero-height detent" as closed. Reading the - * detent's value rather than assuming index `0` keeps this right for sheets - * whose collapsed detent isn't first, and for `expandedIndex` pointing at a - * middle detent. + * The manager treats "settled on a zero-height detent" as closed. */ function isClosedDetent(detents: Detent[], index: number): boolean { const detent = detents[index]; diff --git a/src/adapters/swmansion/index.ts b/src/adapters/swmansion/index.ts index 26dee38..d3edeed 100644 --- a/src/adapters/swmansion/index.ts +++ b/src/adapters/swmansion/index.ts @@ -3,8 +3,6 @@ export { programmatic, type SwmansionSheetAdapterProps, type SwmansionHandleConfig, - type SwmansionScrollableNegotiation, - type SwmansionScrollableNegotiationMode, type Detent, type DetentValue, } from './SwmansionSheetAdapter'; diff --git a/src/animatedRegistry.ts b/src/animatedRegistry.ts index ffe04b1..4a002d0 100644 --- a/src/animatedRegistry.ts +++ b/src/animatedRegistry.ts @@ -2,8 +2,12 @@ import { makeMutable, type SharedValue } from 'react-native-reanimated'; /** * Registry for shared animated values per sheet. - * AnimatedIndex is created eagerly in store actions (open/mount) - * before any component renders, ensuring it's always available. + * + * Keyed by sheet ID rather than held in the store: shared values are not + * serializable state, and both the store actions and the rendering hooks need + * to reach the same value for a sheet. Either side may be first to ask for it, + * so entries are created on demand — a sheet that has been cleaned up has no + * entry, and callers handle its absence. */ const animatedIndexRegistry = new Map>(); @@ -39,16 +43,6 @@ export function getAnimatedIndex( return animatedIndexRegistry.get(sheetId); } -/** - * Set the animated index value for a sheet. - */ -export function setAnimatedIndexValue(sheetId: string, value: number): void { - const animatedIndex = animatedIndexRegistry.get(sheetId); - if (animatedIndex) { - animatedIndex.value = value; - } -} - export function cleanupAnimatedIndex(sheetId: string): void { animatedIndexRegistry.delete(sheetId); } diff --git a/src/bottomSheetCoordinator.ts b/src/bottomSheetCoordinator.ts index 520c70a..e0c7b2f 100644 --- a/src/bottomSheetCoordinator.ts +++ b/src/bottomSheetCoordinator.ts @@ -10,7 +10,7 @@ import { getSheetRef } from './refsMap'; * The store can reach a terminal status before the adapter has mounted — a * portal sheet has to teleport its content into the `PortalHost` first. A * single attempt would be a silent no-op, leaving the sheet stuck in that - * status forever (and, for 'closing', blocking every later open in the group). + * status forever (and, for 'opening', blocking every later open in the group). */ const REF_CALL_MAX_FRAMES = 10; @@ -43,10 +43,13 @@ function driveSheetRef( console.warn( `[BottomSheet] Sheet "${id}" reached status "${expectedStatus}" but its ` + 'adapter never registered a ref, so the transition could not be driven. ' + - 'The sheet will be stuck in this status. Make sure the adapter forwards ' + - 'its ref (see useAdapterRef).' + 'The sheet has been forced to a closed state so the rest of its group ' + + 'keeps working. Make sure the adapter forwards its ref (see useAdapterRef).' ); } + // Without a terminal status an 'opening' sheet blocks every later open in + // its group, with no way back short of destroyAll(). + useBottomSheetStore.getState().finishClosing(id); return; } @@ -100,12 +103,11 @@ export function initBottomSheetCoordinator(groupId: string) { * answers that callers routinely need to tell apart. */ export async function requestClose(sheetId: string): Promise { - const state = useBottomSheetStore.getState(); - const currentStatus = state.sheetsById[sheetId]?.status; + const initialStatus = + useBottomSheetStore.getState().sheetsById[sheetId]?.status; - // Don't run interceptor if sheet is already closing // This prevents duplicate interceptor calls during close animations - if (currentStatus === 'closing') { + if (initialStatus === 'closing') { return { closed: false, reason: 'not-closable' }; } @@ -119,18 +121,17 @@ export async function requestClose(sheetId: string): Promise { onCancel: () => resolve(false), }); - if (result) { - if (typeof result === 'boolean') { - resolve(result); - } else if ( - result && - typeof result === 'object' && - 'then' in result && - typeof result.then === 'function' - ) { - // It's a Promise - result.then(resolve); - } + // Discriminated on type, not truthiness: `false` is the documented way + // to block, and a truthiness check would drop it and never settle. + if (typeof result === 'boolean') { + resolve(result); + } else if ( + result && + typeof result === 'object' && + 'then' in result && + typeof result.then === 'function' + ) { + result.then(resolve); } }); @@ -138,7 +139,6 @@ export async function requestClose(sheetId: string): Promise { return { closed: false, reason: 'blocked' }; } } catch (error) { - // If the interceptor throws, cancel the close for safety if (__DEV__) { console.warn( `[BottomSheet] onBeforeClose interceptor threw an error for sheet "${sheetId}". ` + @@ -150,6 +150,12 @@ export async function requestClose(sheetId: string): Promise { } } + // Re-read rather than reusing the pre-interceptor snapshot: awaiting the + // interceptor can mean awaiting a user, and the sheet may have been closed, + // cleared or re-opened in the meantime. + const state = useBottomSheetStore.getState(); + const currentStatus = state.sheetsById[sheetId]?.status; + if (currentStatus === 'open' || currentStatus === 'opening') { state.startClosing(sheetId); return { closed: true }; @@ -189,7 +195,6 @@ export async function closeAllAnimated( const state = useBottomSheetStore.getState(); - // Close from top to bottom (reverse order) const reversed = [...(state.stackOrderByGroup[groupId] ?? [])].reverse(); const closed: string[] = []; diff --git a/src/refsMap.ts b/src/refsMap.ts index 142b5bf..4f39f5c 100644 --- a/src/refsMap.ts +++ b/src/refsMap.ts @@ -21,3 +21,11 @@ export function cleanupSheetRef(sheetId: string): void { export function __resetSheetRefs(): void { sheetRefsMap.clear(); } + +/** + * Get all sheet refs for debugging. + * @internal + */ +export function __getAllSheetRefs(): Map { + return sheetRefsMap; +} diff --git a/src/store/helpers.ts b/src/store/helpers.ts index 1f8927d..3a352d5 100644 --- a/src/store/helpers.ts +++ b/src/store/helpers.ts @@ -1,4 +1,9 @@ -import type { BottomSheetState, BottomSheetStatus, OpenMode } from './types'; +import type { + BottomSheetState, + BottomSheetStatus, + BottomSheetStoreState, + OpenMode, +} from './types'; /** * Status to force onto the previous top sheet when a new one opens. @@ -77,6 +82,39 @@ export function getGroupStack( return stackOrderByGroup[groupId] ?? []; } +/** + * Takes `id` off its group's stack and brings the sheet it was covering back. + * + * A sheet parked as `hidden` by `switch` is still on the stack, so whatever + * removes the sheet above it — a finished close or an unmount — has to send it + * to `opening`, or the group is left with a top sheet that renders as active + * while actually being closed. + */ +export function detachFromGroup( + sheetsById: Record, + stackOrderByGroup: Record, + groupId: string, + id: string +): BottomSheetStoreState { + const newGroupStack = removeFromStack( + getGroupStack(stackOrderByGroup, groupId), + id + ); + const topId = getTopSheetId(newGroupStack); + + return { + sheetsById: + topId && isHidden(sheetsById[topId]) + ? updateSheet(sheetsById, topId, { status: 'opening' }) + : sheetsById, + stackOrderByGroup: withGroupStack( + stackOrderByGroup, + groupId, + newGroupStack + ), + }; +} + /** * Returns `stackOrderByGroup` with `groupId`'s stack replaced, dropping the key * once its stack is empty so groups don't accumulate forever. diff --git a/src/store/store.ts b/src/store/store.ts index aa6ce52..17bbc90 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -3,6 +3,7 @@ import { createWithEqualityFn as create } from 'zustand/traditional'; import { applyModeToTopSheet, + detachFromGroup, getGroupStack, getSheetBelowId, getTopSheetId, @@ -17,6 +18,7 @@ import { getNextPortalSession } from '../portalSessionRegistry'; import type { BottomSheetState, BottomSheetStore, + BottomSheetStoreState, OpenPayload, OpenRejectionReason, OpenResult, @@ -38,12 +40,27 @@ function toStoreFields(sheet: OpenPayload) { function warnRejectedOpen(id: string, reason: OpenRejectionReason) { if (!__DEV__) return; - const explanation = - reason === 'already-active' - ? `Sheet "${id}" is already on the stack. Re-opening an active sheet is a no-op by design; close it first, or use updateParams() to change its content.` - : `Sheet "${id}" was not opened because another sheet in the same group is still animating open. Wait for it to settle (useBottomSheetStatus) before opening the next one.`; + const explanations: Record = { + 'already-active': `Sheet "${id}" is already on the stack. Re-opening an active sheet is a no-op by design; close it first, or use updateParams() to change its content.`, + 'group-busy': `Sheet "${id}" was not opened because another sheet in the same group is still animating open. Wait for it to settle (useBottomSheetStatus) before opening the next one.`, + 'group-mismatch': `Sheet "${id}" is registered to a different manager group than the one opening it. A sheet belongs to the group that mounted it; declare a separate sheet in the other group instead.`, + }; - console.warn(`[BottomSheet] open() ignored. ${explanation}`); + console.warn(`[BottomSheet] open() ignored. ${explanations[reason]}`); +} + +/** + * Guard-then-patch used by every "change one field on an existing sheet" + * action: a write for an id the store no longer knows must leave the state + * object untouched, so subscribers are not woken by a no-op. + */ +function patchSheet( + state: BottomSheetStoreState, + id: string, + update: Partial +): Partial { + if (!state.sheetsById[id]) return state; + return { sheetsById: updateSheet(state.sheetsById, id, update) }; } export const useBottomSheetStore = create( @@ -62,6 +79,11 @@ export const useBottomSheetStore = create( return { opened: false, id: sheet.id, reason: 'already-active' }; } + if (existingSheet && existingSheet.groupId !== sheet.groupId) { + warnRejectedOpen(sheet.id, 'group-mismatch'); + return { opened: false, id: sheet.id, reason: 'group-mismatch' }; + } + const hasOpeningInGroup = Object.values(state.sheetsById).some( (s) => s.groupId === sheet.groupId && s.status === 'opening' ); @@ -70,6 +92,18 @@ export const useBottomSheetStore = create( return { opened: false, id: sheet.id, reason: 'group-busy' }; } + const fields = toStoreFields(sheet); + + // Past the guards an existing record is always a persistent sheet, and + // its session was allocated once at mount() — only a fresh portal sheet + // needs one. + const portalSession = + fields.usePortal && !existingSheet + ? getNextPortalSession(sheet.id) + : undefined; + + resetAnimatedIndex(sheet.id); + set((current) => { const groupStack = getGroupStack( current.stackOrderByGroup, @@ -82,16 +116,6 @@ export const useBottomSheetStore = create( mode ); - const fields = toStoreFields(sheet); - - const shouldGetNewPortalSession = - fields.usePortal && (!existingSheet || !existingSheet.keepMounted); - const nextPortalSession = shouldGetNewPortalSession - ? getNextPortalSession(sheet.id) - : undefined; - - resetAnimatedIndex(sheet.id); - const newSheet: BottomSheetState = existingSheet ? { ...existingSheet, @@ -100,18 +124,17 @@ export const useBottomSheetStore = create( sheet.scaleBackground ?? existingSheet.scaleBackground, backdrop: sheet.backdrop ?? existingSheet.backdrop, params: sheet.params ?? existingSheet.params, - portalSession: existingSheet.keepMounted - ? existingSheet.portalSession - : (nextPortalSession ?? existingSheet.portalSession), } - : { ...fields, status: 'opening', portalSession: nextPortalSession }; + : { ...fields, status: 'opening', portalSession }; return { sheetsById: { ...updatedSheetsById, [sheet.id]: newSheet }, stackOrderByGroup: withGroupStack( current.stackOrderByGroup, sheet.groupId, - [...groupStack, sheet.id] + // Re-appended rather than pushed: a sheet parked as `hidden` by + // `switch` is still on the stack, and pushing would duplicate it. + [...removeFromStack(groupStack, sheet.id), sheet.id] ), }; }); @@ -119,13 +142,7 @@ export const useBottomSheetStore = create( return { opened: true, id: sheet.id }; }, - markOpen: (id) => - set((state) => { - if (!state.sheetsById[id]) return state; - return { - sheetsById: updateSheet(state.sheetsById, id, { status: 'open' }), - }; - }), + markOpen: (id) => set((state) => patchSheet(state, id, { status: 'open' })), startClosing: (id) => set((state) => { @@ -136,15 +153,20 @@ export const useBottomSheetStore = create( status: 'closing', }); + // Only the top of the group uncovers anything. Restoring from further + // down would animate a switched-away sheet back in *underneath* the + // current top, and leave the group wedged on the `opening` guard. const groupStack = getGroupStack( state.stackOrderByGroup, sheet.groupId ); - const belowId = getSheetBelowId(groupStack, id); - if (belowId && isHidden(updatedSheetsById[belowId])) { - updatedSheetsById = updateSheet(updatedSheetsById, belowId, { - status: 'opening', - }); + if (getTopSheetId(groupStack) === id) { + const belowId = getSheetBelowId(groupStack, id); + if (belowId && isHidden(updatedSheetsById[belowId])) { + updatedSheetsById = updateSheet(updatedSheetsById, belowId, { + status: 'opening', + }); + } } return { sheetsById: updatedSheetsById }; @@ -165,52 +187,22 @@ export const useBottomSheetStore = create( delete updatedSheetsById[id]; } - const groupStack = getGroupStack( + return detachFromGroup( + updatedSheetsById, state.stackOrderByGroup, - sheet.groupId + sheet.groupId, + id ); - const newGroupStack = removeFromStack(groupStack, id); - const topId = getTopSheetId(newGroupStack); - - if (topId && isHidden(updatedSheetsById[topId])) { - updatedSheetsById = updateSheet(updatedSheetsById, topId, { - status: 'opening', - }); - } - - return { - sheetsById: updatedSheetsById, - stackOrderByGroup: withGroupStack( - state.stackOrderByGroup, - sheet.groupId, - newGroupStack - ), - }; }), updateParams: (id, params) => - set((state) => { - if (!state.sheetsById[id]) return state; - return { sheetsById: updateSheet(state.sheetsById, id, { params }) }; - }), + set((state) => patchSheet(state, id, { params })), setPreventDismiss: (id, prevent) => - set((state) => { - if (!state.sheetsById[id]) return state; - return { - sheetsById: updateSheet(state.sheetsById, id, { - preventDismiss: prevent, - }), - }; - }), + set((state) => patchSheet(state, id, { preventDismiss: prevent })), setBackdrop: (id, backdrop) => - set((state) => { - if (!state.sheetsById[id]) return state; - return { - sheetsById: updateSheet(state.sheetsById, id, { backdrop }), - }; - }), + set((state) => patchSheet(state, id, { backdrop })), clearGroup: (groupId) => set((state) => { @@ -268,19 +260,12 @@ export const useBottomSheetStore = create( const updatedSheetsById = { ...state.sheetsById }; delete updatedSheetsById[id]; - const groupStack = getGroupStack( + return detachFromGroup( + updatedSheetsById, state.stackOrderByGroup, - sheet.groupId + sheet.groupId, + id ); - - return { - sheetsById: updatedSheetsById, - stackOrderByGroup: withGroupStack( - state.stackOrderByGroup, - sheet.groupId, - removeFromStack(groupStack, id) - ), - }; }), })) ); diff --git a/src/store/types.ts b/src/store/types.ts index 0277cfc..b68ecc9 100644 --- a/src/store/types.ts +++ b/src/store/types.ts @@ -21,9 +21,10 @@ export interface BottomSheetState { params?: Record; keepMounted?: boolean; /** - * Incremented each time a portal-based sheet is opened. - * Used to create unique Portal/PortalHost names to work around - * react-native-teleport connection issues after replace flows. + * Allocated per portal connection: once at `mount()` for a persistent sheet, + * and on every open for a non-persistent portal sheet. Used to create unique + * Portal/PortalHost names to work around react-native-teleport connection + * issues after replace flows. */ portalSession?: number; /** @@ -86,7 +87,12 @@ export type OpenRejectionReason = /** The sheet is already on the stack — re-opening an open sheet is a no-op. */ | 'already-active' /** Another sheet in the same group is still animating open. */ - | 'group-busy'; + | 'group-busy' + /** + * The sheet is registered to a different group than the one opening it. A + * sheet belongs to the group that mounted it; moving it is not supported. + */ + | 'group-mismatch'; /** * Outcome of an `open()` call. `opened: false` means the store deliberately diff --git a/src/testing.ts b/src/testing.ts index 0b8f991..2f4b558 100644 --- a/src/testing.ts +++ b/src/testing.ts @@ -15,7 +15,7 @@ import { } from './animatedRegistry'; import { __resetOnBeforeClose } from './onBeforeCloseRegistry'; import { __resetPortalSessions } from './portalSessionRegistry'; -import { __resetSheetRefs } from './refsMap'; +import { __getAllSheetRefs, __resetSheetRefs } from './refsMap'; import { useBottomSheetStore } from './store'; /** @@ -34,10 +34,11 @@ export function resetBottomSheetRegistries(): void { __resetOnBeforeClose(); } -export { - __resetSheetRefs, - __resetAnimatedIndexes, - __getAllAnimatedIndexes, - __resetPortalSessions, - __resetOnBeforeClose, -}; +/** + * Inspectors for the registries a test is most likely to assert on — chiefly + * "did this sheet leave a ref (or a shared value) behind after it closed?". + * + * Read-only views of the live maps; reset through + * {@link resetBottomSheetRegistries} rather than mutating them. + */ +export { __getAllSheetRefs, __getAllAnimatedIndexes }; diff --git a/src/useBackHandler.ts b/src/useBackHandler.ts index 0725ec5..4ff0add 100644 --- a/src/useBackHandler.ts +++ b/src/useBackHandler.ts @@ -2,6 +2,7 @@ import { useEffect } from 'react'; import { BackHandler } from 'react-native'; import { useIsTopmostAndOpen } from './store'; +import { useStableCallback } from './useStableCallback'; /** * Manages Android hardware back button for a sheet. @@ -11,6 +12,11 @@ import { useIsTopmostAndOpen } from './store'; */ export function useBackHandler(id: string, onBackPress: () => void): void { const isTopAndOpen = useIsTopmostAndOpen(id); + // Adapters build their handler from `createSheetEventHandlers(id)` during + // render, so it is a new function every time. Stabilising it keeps the native + // listener subscribed for as long as the sheet is on top, instead of being + // torn down and re-added on every render. + const stableOnBackPress = useStableCallback(onBackPress); useEffect(() => { if (!isTopAndOpen) { @@ -19,10 +25,10 @@ export function useBackHandler(id: string, onBackPress: () => void): void { const subscription = BackHandler.addEventListener( 'hardwareBackPress', () => { - onBackPress(); + stableOnBackPress(); return true; } ); return () => subscription.remove(); - }, [isTopAndOpen, onBackPress]); + }, [isTopAndOpen, stableOnBackPress]); } diff --git a/src/useBottomSheetManager.tsx b/src/useBottomSheetManager.tsx index 8b7e519..d22c4cd 100644 --- a/src/useBottomSheetManager.tsx +++ b/src/useBottomSheetManager.tsx @@ -1,6 +1,7 @@ import React from 'react'; import { useOpen, useClearGroup, type OpenMode } from './store'; +import type { CloseResult } from './store'; import { useMaybeBottomSheetManagerContext } from './BottomSheetManager.context'; import type { SheetAdapterRef } from './adapter.types'; import { closeAllAnimated, requestClose } from './bottomSheetCoordinator'; @@ -74,10 +75,10 @@ export const useBottomSheetManager = () => { /** * Closes a sheet. * - * @returns `true` once the sheet is closing, `false` if an `onBeforeClose` - * interceptor blocked it or there was nothing to close. + * @returns A `CloseResult` — `{ closed: true }`, or `{ closed: false, reason }` + * naming why not (`'blocked'`, `'interceptor-error'`, `'not-closable'`). */ - const close = (id: string) => requestClose(id); + const close = (id: string): Promise => requestClose(id); const closeAll = (options?: CloseAllOptions) => { const groupId = bottomSheetManagerContext?.groupId || 'default'; diff --git a/src/useOnBeforeClose.ts b/src/useOnBeforeClose.ts index 20fd4ae..2a96d5b 100644 --- a/src/useOnBeforeClose.ts +++ b/src/useOnBeforeClose.ts @@ -15,25 +15,8 @@ import { useStableCallback } from './useStableCallback'; * 2. Intercepts all programmatic close paths (backdrop tap, back button, * `close()`, `closeAll()`) and calls the callback first. * - * The interceptor receives `onConfirm` and `onCancel` callbacks. Call these - * when the user makes a decision. This works seamlessly with `Alert.alert`: - * - * ```tsx - * useOnBeforeClose(({ onConfirm, onCancel }) => { - * if (dirty) { - * Alert.alert('Discard changes?', '', [ - * { text: 'Cancel', onPress: onCancel }, - * { text: 'Discard', onPress: onConfirm }, - * ]); - * } else { - * onConfirm(); // Allow close immediately - * } - * }); - * ``` - * - * For backward compatibility, you can still return `boolean` or `Promise`: - * - Return `false` (or resolve to `false`) to prevent closing - * - Return `true` (or resolve to `true`) to allow closing + * The interceptor receives `onConfirm` and `onCancel` and resolves whenever one + * of them is called, so an asynchronous prompt needs no plumbing of its own. * * Use `forceClose()` from `useBottomSheetContext` to bypass the interceptor entirely. * @@ -57,14 +40,14 @@ import { useStableCallback } from './useStableCallback'; * } * ``` * - * @example Boolean return (backward compatible) + * @example Boolean return — an alternative for decisions that need no prompt. + * Returning `boolean` or `Promise` works in place of the callbacks: + * `false` blocks the close, `true` allows it. * ```tsx * function MySheet() { * const [dirty, setDirty] = useState(false); * - * useOnBeforeClose(() => { - * return !dirty; // false blocks, true allows - * }); + * useOnBeforeClose(() => !dirty); * } * ``` */ diff --git a/src/useScaleAnimation.ts b/src/useScaleAnimation.ts index 73ce412..1a2ea32 100644 --- a/src/useScaleAnimation.ts +++ b/src/useScaleAnimation.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useState } from 'react'; import { useAnimatedStyle, useDerivedValue, @@ -67,13 +67,8 @@ function useBackgroundScaleDepth(groupId: string): number { * runs on every store change (twice per render under StrictMode), so writing to * a ref inside it would make the result depend on how often it ran. */ -function useSheetScaleDepth( - groupId: string, - sheetId: string | undefined -): number { +function useSheetScaleDepth(groupId: string, sheetId: string): number { const liveDepth = useBottomSheetStore((state) => { - if (!sheetId) return 0; - const groupStack = state.stackOrderByGroup[groupId] ?? []; const sheetIndex = groupStack.indexOf(sheetId); @@ -96,11 +91,9 @@ function useSheetScaleDepth( }); const [heldDepth, setHeldDepth] = useState(0); - const heldDepthRef = useRef(0); useEffect(() => { - if (liveDepth !== null && liveDepth !== heldDepthRef.current) { - heldDepthRef.current = liveDepth; + if (liveDepth !== null) { setHeldDepth(liveDepth); } }, [liveDepth]); diff --git a/src/useSheetRenderData.ts b/src/useSheetRenderData.ts index 3613966..544433d 100644 --- a/src/useSheetRenderData.ts +++ b/src/useSheetRenderData.ts @@ -8,8 +8,11 @@ export interface SheetRenderItem { } /** - * Deep comparison for SheetRenderItem arrays. - * Returns true if arrays have same items with same values. + * Equality comparator for the selector below. + * + * The selector builds a fresh array on every store write, so reference equality + * never holds and the host would re-render (remounting nothing, but re-running + * every `QueueItem`) on state changes that do not affect what is rendered. */ function sheetRenderDataEqual( a: SheetRenderItem[], @@ -39,8 +42,8 @@ function sheetRenderDataEqual( * unmounting/remounting when a sheet transitions between states. * * Render order: - * 1. Hidden persistent sheets (keepMounted=true, not in stack) - * 2. Active sheets (in stackOrder) + * 1. Hidden persistent sheets (keepMounted=true, not in the group's stack) + * 2. Active sheets (in the group's stack) */ export function useSheetRenderData(): SheetRenderItem[] { const { groupId } = useBottomSheetManagerContext(); @@ -91,8 +94,8 @@ function getActiveSheets( }, groupId: string ): SheetRenderItem[] { - // Already scoped to the group, so no filtering is needed — and stackIndex is - // now per-group, which is what the z-index layering wants. + // The stack is stored per group, so the index is already the sheet's depth + // within its own group — which is what the z-index layering wants. return (state.stackOrderByGroup[groupId] ?? []).map((id, index) => ({ id, stackIndex: index, diff --git a/src/useStableCallback.ts b/src/useStableCallback.ts index 1f80851..387bd33 100644 --- a/src/useStableCallback.ts +++ b/src/useStableCallback.ts @@ -19,6 +19,8 @@ export const useStableCallback = (callback: T) => { callbackRef.current = callback; }); + // The one sanctioned `useCallback` in the codebase: identity *is* the feature + // here, not an optimisation the compiler could reproduce. return useCallback((...args: Parameters): ReturnType => { return callbackRef.current(...args); }, []); From 08cbdc5d212846cc045df9943cde6750babf88d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 5 Aug 2026 06:26:45 +0000 Subject: [PATCH 09/13] docs: correct samples and API references against the current code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A doc audit compared every page against src/ and found samples that no longer compile and tables describing props that do not exist. Broken samples removed or fixed: adapters.md still documented importing the deleted BottomSheetManaged alias; api/types.md carried a whole BottomSheetRef section for a removed export; several pages destructured `clear` from useBottomSheetManager (now destroyAll) or passed open()'s `string | null` straight into APIs that reject null; persistent-sheets.md called open() without params for a registry entry that declares them, so it failed against its own page; type-safe-ids.md and api/hooks.md accessed `params.x` without optional chaining, which cannot typecheck since BottomSheetPortalParams always unions undefined. context-preservation.md branched on isOpen to choose between updating params and opening — with isOpen narrowed to 'open', a second tap during the opening animation took the else branch and was rejected as already-active, silently dropping the update. Uses isVisible now. Corrected against the source: the store diagram still showed a flat stackOrder; intro.md counted four adapters; api/types.md listed a preventDismiss row four lines after saying it had been removed, and typed UseBottomSheetControlReturn's close/closeAll as void; the react-native-modal page documented a backdropOpacity default that does not exist and could not apply since hasBackdrop is forced off; both that page and actions-sheet.md claimed all library props pass through when several are omitted. Added: the five result types (OpenResult, CloseResult, CloseAllResult and their reason unions) that were referenced but never defined, the adapter hooks missing from the hook inventory, `params` in the open() options table, and the /testing subpath, which shipped as a public entry point documented nowhere user-facing. --- docs/docs/adapters.md | 14 +-- docs/docs/api/components.md | 5 +- docs/docs/api/hooks.md | 85 ++++++++++++++--- docs/docs/api/types.md | 93 +++++++++++++++++-- docs/docs/built-in-adapters/actions-sheet.md | 36 ++++++- docs/docs/built-in-adapters/gorhom.md | 26 +++++- .../built-in-adapters/react-native-modal.md | 38 +++++++- docs/docs/close-interception.md | 66 +++++++++++-- docs/docs/context-preservation.md | 15 ++- docs/docs/custom-adapters.md | 39 +++++++- docs/docs/getting-started.md | 20 +++- docs/docs/intro.md | 2 +- docs/docs/persistent-sheets.md | 26 ++++++ docs/docs/type-safe-ids.md | 8 +- 14 files changed, 411 insertions(+), 62 deletions(-) diff --git a/docs/docs/adapters.md b/docs/docs/adapters.md index b70056f..6cbeade 100644 --- a/docs/docs/adapters.md +++ b/docs/docs/adapters.md @@ -1,5 +1,5 @@ --- -sidebar_position: 9 +sidebar_position: 10 --- # Library-Agnostic Architecture @@ -15,7 +15,7 @@ The stack manager controls **when** sheets open and close. Adapters control **ho │ Stack Manager (core) │ │ ┌─────────────────────────────────────────┐ │ │ │ Zustand Store │ │ -│ │ - stackOrder, sheetsById │ │ +│ │ - sheetsById, stackOrderByGroup │ │ │ │ - push / switch / replace │ │ │ │ - scale animations, portals │ │ │ └─────────────┬───────────────────────────┘ │ @@ -108,13 +108,3 @@ Third-party adapters are shipped as separate [subpath exports](https://nodejs.or ### Custom You can [build your own adapter](/custom-adapters) for any overlay library. - -## Backward Compatibility - -`BottomSheetManaged` is available as a deprecated re-export from the gorhom subpath: - -```tsx -// These are equivalent: -import { GorhomSheetAdapter } from 'react-native-bottom-sheet-stack/gorhom'; -import { BottomSheetManaged } from 'react-native-bottom-sheet-stack/gorhom'; -``` diff --git a/docs/docs/api/components.md b/docs/docs/api/components.md index 1d3f7ca..e2485d0 100644 --- a/docs/docs/api/components.md +++ b/docs/docs/api/components.md @@ -72,7 +72,10 @@ Adapters with 3rd-party dependencies are shipped as separate subpath exports: | `ActionsSheetAdapter` | `react-native-bottom-sheet-stack/actions-sheet` | `react-native-actions-sheet` | [ActionsSheetAdapter](/built-in-adapters/actions-sheet) | | `SwmansionSheetAdapter` | `react-native-bottom-sheet-stack/swmansion` | `@swmansion/react-native-bottom-sheet` | [SwmansionSheetAdapter](/built-in-adapters/swmansion) | -:::tip::: +:::tip +Each sheet in the stack picks its own adapter — bottom sheets and modals can be +mixed freely in one stack. +::: See [Library-Agnostic Architecture](/adapters) for how adapters work, or [Building Custom Adapters](/custom-adapters) to create your own. diff --git a/docs/docs/api/hooks.md b/docs/docs/api/hooks.md index fa7bce2..fd3fb64 100644 --- a/docs/docs/api/hooks.md +++ b/docs/docs/api/hooks.md @@ -4,7 +4,10 @@ sidebar_position: 2 # Hooks -Hooks are divided into two categories based on where they can be used: +Hooks fall into two groups: the ones you use to drive sheets from your app, and +the ones an [adapter](/custom-adapters) uses to wire a UI library into the stack. + +### App hooks | Hook | Where to use | Purpose | |------|--------------|---------| @@ -14,6 +17,19 @@ Hooks are divided into two categories based on where they can be used: | `useBottomSheetContext` | **Inside sheet only** | Access current sheet's state and params | | `useOnBeforeClose` | **Inside sheet only** | Intercept close and optionally prevent it | +### Adapter hooks + +Exported for [custom adapter authors](/custom-adapters) — every shipped adapter +is built from these. You do not need them to use the library. + +| Hook | Where to use | Purpose | +|------|--------------|---------| +| `useAdapterRef` | **Inside adapter only** | Resolve the right ref for inline/portal/persistent mode | +| `useAnimatedIndex` | **Inside adapter only** | The sheet's `animatedIndex` shared value, driving backdrop and scale | +| `useBackHandler` | **Inside adapter only** | Android back button, scoped to the topmost open sheet **of its own group** | +| `useSetBackdrop` | Anywhere | Returns `setBackdrop(id, boolean)` — suppress the manager's shared backdrop for a sheet that renders its own | +| `useSheetPreventDismiss` | Anywhere | `useSheetPreventDismiss(id)` — whether an interceptor is currently blocking dismissal, so the adapter can disable native gestures | + --- ## useBottomSheetManager @@ -56,10 +72,11 @@ await closeAll({ stagger: 0 }); ```tsx open(, { - id: 'my-sheet-id', // Custom ID (optional) - groupId: 'my-group', // Custom group (optional) - mode: 'push', // 'push' | 'switch' | 'replace' - scaleBackground: true, // Enable scale animation + id: 'my-sheet-id', // Custom ID (optional) + groupId: 'my-group', // Custom group (optional) + mode: 'push', // 'push' | 'switch' | 'replace' + scaleBackground: true, // Enable scale animation + params: { userId: '1' }, // Readable via useBottomSheetContext() }); ``` @@ -69,7 +86,8 @@ open(, { | `groupId` | `string` | context or `'default'` | Group ID for the sheet | | `mode` | `OpenMode` | `'push'` | Navigation mode | | `scaleBackground` | `boolean` | `false` | Enable background scaling | -| `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. Built-in adapters set this automatically when you give them their own backdrop (e.g. a custom gorhom `backdropComponent`), so you rarely set it by hand. | +| `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. `GorhomSheetAdapter` sets this itself when you pass it a custom `backdropComponent`; the other shipped adapters use the manager's backdrop and never touch it. | +| `params` | `Record` | - | Params for the sheet, readable inside it via `useBottomSheetContext()`. Untyped here — the typed variant lives on `useBottomSheetControl` | `open()` returns the sheet's ID, or **`null`** when the store declined to open it — because the sheet is already on the stack, or another sheet in the group is still animating open. A dev-mode warning explains which. @@ -138,9 +156,16 @@ Pass the portal sheet ID as a generic to get typed params: ```tsx // If registry defines: 'user-sheet': { userId: string } const { params } = useBottomSheetContext<'user-sheet'>(); -console.log(params.userId); // type-safe: string +console.log(params?.userId); // type-safe: string | undefined ``` +:::note Always optional +`BottomSheetPortalParams` resolves to `{ userId: string } | undefined`, even +when the registry marks the params as required. `resetParams()` can clear them +while the sheet is open, so the sheet must handle their absence — under `strict`, +`params.userId` is a type error. Read them with `params?.userId`. +::: + ### Returns | Property | Type | Description | @@ -200,7 +225,7 @@ open({ |--------|------|---------|-------------| | `mode` | `OpenMode` | `'push'` | Navigation mode | | `scaleBackground` | `boolean` | `false` | Enable background scaling | -| `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. Built-in adapters set this automatically when given their own native backdrop/scrim, so you rarely set it by hand. | +| `backdrop` | `boolean` | `true` | When `false`, the manager's shared backdrop is not rendered for this sheet. `GorhomSheetAdapter` sets this itself when you pass it a custom `backdropComponent`; the other shipped adapters use the manager's backdrop and never touch it. | | `params` | `BottomSheetPortalParams` | - | Type-safe params | `useBottomSheetManager().open()` also accepts `params` now, so inline sheets can read them from `useBottomSheetContext()` just like portal sheets. @@ -221,9 +246,15 @@ const { status, isOpen } = useBottomSheetStatus('my-sheet'); // Inline sheet (using ID from open()) const { open } = useBottomSheetManager(); -const sheetId = open(); -// ... -const { status, isOpen } = useBottomSheetStatus(sheetId); +const [sheetId, setSheetId] = useState(null); + +const handleOpen = () => { + // open() returns `string | null` — null means the store declined + setSheetId(open()); +}; + +// The hook needs a string, so fall back to an ID that matches nothing +const { status, isOpen } = useBottomSheetStatus(sheetId ?? ''); ``` ### Parameters @@ -342,3 +373,35 @@ When active, the hook: Use `forceClose()` from `useBottomSheetContext` to bypass the interceptor entirely. See [Close Interception](/close-interception) for detailed guide and examples. + +--- + +## Testing + +Test helpers ship on the `react-native-bottom-sheet-stack/testing` subpath, so +they stay out of your production bundle. + +```tsx +import { resetBottomSheetRegistries } from 'react-native-bottom-sheet-stack/testing'; + +beforeEach(resetBottomSheetRegistries); +``` + +### resetBottomSheetRegistries + +Clears the store **and** every module-level registry the library keeps: sheet +refs, animated index values, portal sessions and `onBeforeClose` interceptors. + +Those registries are module state, so they outlive React. Without this, a test +that opens a sheet leaves its ref and animated value behind for the next test, +which then sees a sheet it never opened. Prefer this one call over resetting +registries by hand — it cannot go out of date as registries are added. + +| Export | Type | Description | +|--------|------|-------------| +| `resetBottomSheetRegistries` | `() => void` | Clears the store and all registries | + +:::warning Tests only +Nothing on this subpath is meant for application code. It clears state without +running animations or `onBeforeClose` interceptors. +::: diff --git a/docs/docs/api/types.md b/docs/docs/api/types.md index 24354cb..56a9a61 100644 --- a/docs/docs/api/types.md +++ b/docs/docs/api/types.md @@ -61,20 +61,91 @@ interface BottomSheetState { | Property | Type | Description | |----------|------|-------------| | `keepMounted` | `boolean` | When `true`, sheet stays in store after close (persistent mode) | -| `preventDismiss` | `boolean` | When `true`, adapters block native dismiss gestures. Set by `useOnBeforeClose`. | --- -### BottomSheetRef +## Result Types -Backward-compatible alias for `SheetAdapterRef`. Ref type for all adapters. +Both `open()` and `close()` report why they did nothing, rather than failing +silently. See [Close results](/api/hooks#close-results) for how to consume them. + +### OpenResult + +Outcome of an `open()` call on the store. + +```tsx +type OpenResult = + | { opened: true; id: string } + | { opened: false; id: string; reason: OpenRejectionReason }; +``` + +The public hooks project this down to the currency that is useful at each call +site: `useBottomSheetManager().open()` returns `string | null` (the ID, or +`null`), and `useBottomSheetControl().open()` returns `boolean`. + +--- + +### OpenRejectionReason + +Why an `open()` call did not put the sheet on the stack. Each also logs a +`__DEV__` warning explaining which. + +```tsx +type OpenRejectionReason = 'already-active' | 'group-busy' | 'group-mismatch'; +``` + +| Reason | Meaning | +|--------|---------| +| `already-active` | The sheet is already on the stack. Re-opening an active sheet is a no-op — close it first, or use `updateParams()` | +| `group-busy` | Another sheet in the same group is still animating open. Wait for it to settle (`useBottomSheetStatus`) | +| `group-mismatch` | The sheet is registered to a different manager group than the one opening it. A sheet belongs to the group that mounted it | + +--- + +### CloseResult + +Outcome of a close. Returned by `close()` on `useBottomSheetManager`, +`useBottomSheetControl` and `useBottomSheetContext`. ```tsx -import type { BottomSheetRef } from 'react-native-bottom-sheet-stack'; +type CloseResult = + | { closed: true } + | { closed: false; reason: CloseRejectionReason }; +``` + +--- + +### CloseRejectionReason + +```tsx +type CloseRejectionReason = 'blocked' | 'interceptor-error' | 'not-closable'; +``` + +| Reason | Meaning | +|--------|---------| +| `blocked` | A [`useOnBeforeClose`](/close-interception) interceptor declined | +| `interceptor-error` | The interceptor threw; the close is cancelled for safety | +| `not-closable` | There was nothing to close — already closing, hidden, or unknown sheet. Distinct from `blocked`: no interceptor had an opinion | + +--- + +### CloseAllResult + +Outcome of a cascading `closeAll()`. -const sheetRef = useRef(null); +```tsx +interface CloseAllResult { + /** Whether every sheet in the group closed. */ + closedAll: boolean; + /** IDs that closed, topmost first. */ + closed: string[]; + /** The sheet whose interceptor stopped the cascade, if one did. */ + stoppedAt?: string; +} ``` +A sheet with nothing to close does not stop the cascade — only a refusal does. + --- ## Adapter Types @@ -224,6 +295,13 @@ type Params = BottomSheetPortalParams<'settings-sheet'>; // Result: undefined ``` +:::note `undefined` is always in the union +Even for a sheet with required params, the resolved type is `T | undefined` — +`resetParams()` can clear params on an open sheet, so a sheet reading its own +params can always find them missing. Under `strict`, read them optionally: +`params?.userId`. +::: + --- ## Hook Return Types @@ -234,9 +312,10 @@ Return type of `useBottomSheetControl` hook. ```tsx interface UseBottomSheetControlReturn { + /** `false` when the store declined — see OpenRejectionReason. */ open: OpenFunction; - close: () => void; - closeAll: (options?: CloseAllOptions) => Promise; + close: () => Promise; + closeAll: (options?: CloseAllOptions) => Promise; updateParams: (params: BottomSheetPortalParams) => void; resetParams: () => void; } diff --git a/docs/docs/built-in-adapters/actions-sheet.md b/docs/docs/built-in-adapters/actions-sheet.md index ef2e409..342c1c7 100644 --- a/docs/docs/built-in-adapters/actions-sheet.md +++ b/docs/docs/built-in-adapters/actions-sheet.md @@ -29,10 +29,38 @@ function MyActionsSheet() { ## Props -All [`react-native-actions-sheet` props](https://github.com/ammarahm-ed/react-native-actions-sheet#actionsheet-props) are accepted via spread. +`ActionsSheetAdapterProps` is `Omit` — most of +[`react-native-actions-sheet`'s props](https://github.com/ammarahm-ed/react-native-actions-sheet#actionsheet-props) +pass straight through, but five are removed from the type because the manager +owns them. -Adapter defaults (overridable): `gestureEnabled`, `closeOnTouchBackdrop`, `closeOnPressBack`, `keyboardHandlerEnabled`. +**Managed by the adapter (not accepted):** -:::tip -This adapter uses `isModal={false}` internally to avoid wrapping in a redundant Modal — the stack manager handles the overlay lifecycle. +| Prop | Why | +|------|-----| +| `isModal` | Forced `false` — wrapping in a native Modal would take the sheet out of the stack's z-index layering. The manager handles the overlay lifecycle | +| `defaultOverlayOpacity` | Forced `0` — the library paints its own overlay regardless of `isModal`, which would stack on the manager's `BottomSheetBackdrop` as a double-dark layer | +| `onOpen` | Consumed → starts the backdrop fade-in, then `handleOpened()` | +| `onClose` | Consumed → starts the backdrop fade-out, then `handleClosed()` | +| `onBeforeClose` | Consumed → `handleDismiss()` | + +**Adapter defaults (yours wins):** + +| Prop | Default | Note | +|------|---------|------| +| `gestureEnabled` | `true` | Set to `false` while a [`useOnBeforeClose`](/close-interception) interceptor is blocking dismissal | +| `keyboardHandlerEnabled` | `true` | | + +:::info Backdrop timing +`openAnimationConfig` and `closeAnimationConfig` do double duty: the adapter +springs the manager's backdrop with the same config, so the fade rides the +sheet's own curve. `onOpen` / `onClose` fire when the sheet *starts* moving, +which is what lets the two run together. +::: + +:::note Blocked dismissal keeps its escape hatches +Only the swipe gesture is disabled while an interceptor is blocking. Back button +and backdrop tap stay enabled, because they route through `onBeforeClose` into +the manager's interceptor — which is what produces the confirmation prompt. +Disabling them natively would make the sheet silently undismissable. ::: diff --git a/docs/docs/built-in-adapters/gorhom.md b/docs/docs/built-in-adapters/gorhom.md index 1827e11..2ee84e0 100644 --- a/docs/docs/built-in-adapters/gorhom.md +++ b/docs/docs/built-in-adapters/gorhom.md @@ -2,7 +2,11 @@ The default adapter. Wraps `@gorhom/bottom-sheet` to provide feature-rich bottom sheets with snap points, spring animations, and swipe gestures. -:::tip::: +:::tip +`@gorhom/bottom-sheet` is an optional peer dependency — install it only if you +use this adapter. It is imported from the `/gorhom` subpath, never from the +main entry point. +::: ## Installation @@ -32,7 +36,25 @@ const MySheet = forwardRef((props, ref) => { ## Props -Accepts all props from [`@gorhom/bottom-sheet`](https://gorhom.dev/react-native-bottom-sheet/props). The adapter overrides `enablePanDownToClose` to `true` by default. +`GorhomSheetAdapterProps` extends [`BottomSheetProps`](https://gorhom.dev/react-native-bottom-sheet/props) — the full gorhom prop surface is accepted, nothing is omitted from the type. But the manager owns some of it at runtime. + +**Managed by the adapter (your value is ignored or wrapped):** + +| Prop | What the adapter does | +|------|----------------------| +| `index` | Set from the manager: `0` for portal and inline sheets, `-1` for a persistent sheet that is mounted but closed | +| `animatedIndex` | Replaced with the manager's shared value, which drives the backdrop and scale. A value you pass is still **mirrored** — the adapter writes every frame into it, so `animatedIndex` you own keeps working | +| `onChange` | Wrapped — reports `handleOpened()` at index `>= 0`, then calls yours | +| `onClose` | Wrapped — calls yours, then reports `handleClosed()` | +| `onAnimate` | Wrapped — reports `handleDismiss()` when animating toward `-1`, then calls yours | + +**Adapter defaults (yours wins):** + +| Prop | Default | Note | +|------|---------|------| +| `animationConfigs` | spring — `stiffness: 400`, `damping: 80`, `mass: 0.7` | | +| `backdropComponent` | a component returning `null` | See [Backdrop](#backdrop) | +| `enablePanDownToClose` | `true` | Forced to `false` while a [`useOnBeforeClose`](/close-interception) interceptor is blocking dismissal, so the interceptor always gets to run | ## Backdrop diff --git a/docs/docs/built-in-adapters/react-native-modal.md b/docs/docs/built-in-adapters/react-native-modal.md index ab813b2..2b4c0a1 100644 --- a/docs/docs/built-in-adapters/react-native-modal.md +++ b/docs/docs/built-in-adapters/react-native-modal.md @@ -21,7 +21,8 @@ function FancyModal() { animationIn="slideInUp" animationOut="slideOutDown" swipeDirection="down" - backdropOpacity={0.6} + animationInTiming={400} + animationOutTiming={250} > Fancy animated modal @@ -34,6 +35,37 @@ function FancyModal() { ## Props -All [`react-native-modal` props](https://github.com/react-native-modal/react-native-modal#available-props) are accepted via spread. +`ReactNativeModalAdapterProps` is `Partial>` — most of +[`react-native-modal`'s props](https://github.com/react-native-modal/react-native-modal#available-props) +pass straight through, but seven are removed from the type because the manager +owns them. -Adapter defaults (overridable): `swipeDirection="down"`, `backdropOpacity={0.5}`, `useNativeDriver`, `hideModalContentWhileAnimating`. +**Managed by the adapter (not accepted):** + +| Prop | Why | +|------|-----| +| `isVisible` | The manager drives visibility through the adapter ref | +| `coverScreen` | Forced `false`, so the modal renders as a plain `View` and `QueueItem`'s z-index controls stacking in `push` mode | +| `hasBackdrop` | Forced `false` — the manager's stack-aware `BottomSheetBackdrop` provides the overlay | +| `onModalShow` | Consumed → `handleOpened()` | +| `onModalHide` | Consumed → `handleClosed()` | +| `onBackButtonPress` | Consumed → `handleDismiss()` (this adapter uses the library's own back handling rather than the manager's `useBackHandler`) | +| `onSwipeComplete` | Consumed → `handleDismiss()`, and disabled while dismissal is blocked | + +**Adapter defaults (yours wins):** + +| Prop | Default | Note | +|------|---------|------| +| `animationInTiming` | `300` | Also times the manager's backdrop fade-in — see below | +| `animationOutTiming` | `300` | Also times the manager's backdrop fade-out | +| `swipeDirection` | `'down'` | Set to `undefined` while a [`useOnBeforeClose`](/close-interception) interceptor is blocking dismissal | +| `useNativeDriver` | `true` | | +| `hideModalContentWhileAnimating` | `true` | | + +:::info Backdrop timing +Because `hasBackdrop` is forced off, `backdropOpacity`, `backdropColor` and the +other backdrop props have no effect — there is no react-native-modal backdrop to +style. The manager's backdrop is faded with `withTiming` over +`animationInTiming` / `animationOutTiming`, so overriding those keeps the +backdrop in step with the modal instead of letting it run ahead. +::: diff --git a/docs/docs/close-interception.md b/docs/docs/close-interception.md index 486e5e2..4c6a69a 100644 --- a/docs/docs/close-interception.md +++ b/docs/docs/close-interception.md @@ -162,16 +162,62 @@ Result: [SheetA, SheetB] remain open This seamless integration with `closeAll()` is why the callback pattern is recommended over the boolean return pattern. -## Adapter Support +## Observing a blocked close -For `useOnBeforeClose` to fully work, the adapter must respect the `preventDismiss` prop. All built-in adapters support this: +Blocking is not silent — every close path reports what happened, so the caller +can tell "the user declined" from "there was nothing to close": -| Adapter | preventDismiss support | -|---------|----------------------| -| `GorhomSheetAdapter` | `enablePanDownToClose={false}` when active | -| `CustomModalAdapter` | Disables backdrop press | -| `ReactNativeModalAdapter` | Disables swipe and backdrop press | -| `ActionsSheetAdapter` | `closable={false}` when active | -| `SwmansionSheetAdapter` | Re-snaps up when the user swipes to the collapsed detent | +```tsx +const { close, closeAll } = useBottomSheetControl('editor'); + +const result = await close(); +if (!result.closed) { + switch (result.reason) { + case 'blocked': // the interceptor declined + case 'interceptor-error': // the interceptor threw; cancelled for safety + case 'not-closable': // already closing, hidden, or unknown sheet + } +} + +const cascade = await closeAll(); +if (!cascade.closedAll) { + cascade.stoppedAt; // the sheet whose interceptor stopped the cascade + cascade.closed; // the ones that did close, topmost first +} +``` + +`close()` on `useBottomSheetManager`, `useBottomSheetControl` and +`useBottomSheetContext` all resolve to a +[`CloseResult`](/api/types#closeresult); `closeAll()` resolves to a +[`CloseAllResult`](/api/types#closeallresult). + +A sheet that had nothing to close does **not** stop a cascade — only a refusal +does. See [Close results](/api/hooks#close-results). + +## Adapter Support + +For `useOnBeforeClose` to fully work, the adapter has to notice `preventDismiss` +and disable the dismiss paths its library handles natively — otherwise the +library closes the sheet without ever asking the interceptor. + +| Adapter | What it does while dismissal is blocked | +|---------|------------------------------------------| +| `GorhomSheetAdapter` | Forces `enablePanDownToClose={false}` | +| `ReactNativeModalAdapter` | Clears `swipeDirection` and `onSwipeComplete`, so swipe-to-dismiss is inert. Back button still routes through the interceptor | +| `ActionsSheetAdapter` | Sets `gestureEnabled={false}`. Back button and backdrop tap stay enabled — they route through `onBeforeClose` into the interceptor, which is what produces the prompt | +| `SwmansionSheetAdapter` | Rewrites detent `0` to `programmatic()` so the user cannot swipe to it, re-snaps up if the sheet reaches the collapsed detent anyway, and hides the grab handle | +| `CustomModalAdapter` | **Nothing** — see below | + +:::warning `CustomModalAdapter` does not block gestures +It never reads `preventDismiss`. It renders no backdrop of its own and has no +swipe gesture, so its only user-driven dismiss path is the Android back button — +which goes through `handleDismiss()` and therefore still runs the interceptor. +Programmatic `close()` is intercepted as normal. But if you wrap it in your own +tap-to-dismiss surface, that surface must check `preventDismiss` itself. +::: -If you're building a [custom adapter](/custom-adapters), read the `preventDismiss` value from the store and disable native dismiss gestures accordingly. +If you're building a [custom adapter](/custom-adapters), read the flag with the +exported `useSheetPreventDismiss(id)` hook and disable your library's native +dismiss gestures while it is `true`. Inside a sheet, the same value is on +`useBottomSheetContext().preventDismiss` — useful for UI that should reflect it, +such as hiding a grab handle. diff --git a/docs/docs/context-preservation.md b/docs/docs/context-preservation.md index e27abff..6a74b01 100644 --- a/docs/docs/context-preservation.md +++ b/docs/docs/context-preservation.md @@ -143,11 +143,11 @@ const UserSheet = forwardRef((props, ref) => { function UserList() { const { open, updateParams, resetParams } = useBottomSheetControl('user-sheet'); - const { isOpen } = useBottomSheetStatus('user-sheet'); + const { isVisible } = useBottomSheetStatus('user-sheet'); const showUser = (userId: string) => { - if (isOpen) { - // Sheet already open - just update the params + if (isVisible) { + // Sheet already on screen - just update the params updateParams({ userId }); } else { // Open with initial params @@ -173,3 +173,12 @@ function UserList() { ); } ``` + +:::warning Branch on `isVisible`, not `isOpen` +`isOpen` is `true` only once the sheet is **fully** open. A second tap while it +is still animating in would take the `else` branch, and `open()` would reject +the call as `'already-active'` — the params would silently not update. +`isVisible` covers `opening`, `open` and `closing`, which is what "already on +screen, just update it" means. See +[`useBottomSheetStatus`](/api/hooks#usebottomsheetstatus). +::: diff --git a/docs/docs/custom-adapters.md b/docs/docs/custom-adapters.md index a077935..fbfc917 100644 --- a/docs/docs/custom-adapters.md +++ b/docs/docs/custom-adapters.md @@ -30,6 +30,30 @@ interface SheetAdapterEvents { } ``` +It should also: + +3. **Drive `animatedIndex` alongside your own animation**, so the manager's + backdrop fades in step with the sheet rather than snapping — see + [Animated Index](#animated-index) +4. **Respect `preventDismiss`** — read it with `useSheetPreventDismiss(id)` and + disable your library's native dismiss gestures while it is `true`, so a + [`useOnBeforeClose`](/close-interception) interceptor always gets to run + before the sheet goes away. Every shipped adapter except `CustomModalAdapter` + (which has no dismiss gesture of its own) does this. +5. **Handle the Android back button** with `useBackHandler(id, handleDismiss)` + unless your library already has its own back handling to route into + `handleDismiss()`. The hook is scoped to the topmost open sheet **of its own + group**, which a hand-rolled `BackHandler` listener is not. + +```tsx +import { useBackHandler, useSheetPreventDismiss } from 'react-native-bottom-sheet-stack'; + +const preventDismiss = useSheetPreventDismiss(id); +useBackHandler(id, handleDismiss); + + +``` + ## Step-by-Step Guide ### 1. Create the Adapter Component @@ -74,9 +98,13 @@ export const MyAdapter = React.forwardRef( }, }), []); - // 5. Wire up callbacks + // 5. Wire up callbacks. Animate animatedIndex with the same timing as your + // own show/hide animation so the manager's backdrop fades in step. + const onShowStart = () => { + animatedIndex.set(withTiming(0, { duration: 300 })); + }; + const onShown = () => { - animatedIndex.set(0); handleOpened(); }; @@ -84,8 +112,11 @@ export const MyAdapter = React.forwardRef( handleDismiss(); }; + const onHideStart = () => { + animatedIndex.set(withTiming(-1, { duration: 300 })); + }; + const onHidden = () => { - animatedIndex.set(-1); handleClosed(); }; @@ -93,8 +124,10 @@ export const MyAdapter = React.forwardRef( return ( diff --git a/docs/docs/getting-started.md b/docs/docs/getting-started.md index 66eab8f..2fb4176 100644 --- a/docs/docs/getting-started.md +++ b/docs/docs/getting-started.md @@ -13,9 +13,12 @@ yarn add react-native-bottom-sheet-stack ### Core Peer Dependencies ```bash -yarn add react-native-reanimated react-native-safe-area-context react-native-teleport zustand +yarn add react-native-reanimated react-native-safe-area-context react-native-teleport react-native-worklets zustand ``` +`react-native-worklets` is not optional — it backs Reanimated 4 and is imported +directly by the core adapters to hop from a worklet back to the JS thread. + ### Adapter-Specific Dependencies Install only the dependencies for the adapter(s) you plan to use: @@ -123,3 +126,18 @@ function MyComponent() { return