fix(BottomSheetModal): don't fire onDismiss when sheet was never presented#2704
Open
olivier-bouillet wants to merge 1 commit into
Open
Conversation
…ented `unmount()` fired the user-provided `onDismiss` callback unconditionally, even when the modal was never presented. Calling `dismiss()` on a never-presented (e.g. permanently-mounted) modal therefore fired a phantom `onDismiss`. `hadReactMount` is already computed and used to gate the React unmount (`setState`); extend the same guard to the callback so `onDismiss` only fires for a sheet that was actually presented. Closes gorhom#2703
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
BottomSheetModal'sunmount()fires the user-providedonDismisscallback unconditionally, even when the modal was never presented. Callingref.current?.dismiss()on a modal that was neverpresent()-ed therefore triggers a phantomonDismiss.The function already computes
hadReactMountand uses it to gate the React unmount (setState), butonDismissis fired without the same guard — which is inconsistent, sinceonDismissmeans "a presented sheet was dismissed".This breaks consumers that run side effects in
onDismiss(e.g.navigation.goBack()): a phantom dismiss on a permanently-mounted, never-presented modal navigates the user away. Reproducible onreact-native@0.85/ the new architecture.Fixes #2703.
Change
Gate the callback on the already-computed
hadReactMount, mirroring thesetStateguard:One-line behavioral change;
onDismissnow only fires for a sheet that was actually presented.Verification
In an app that permanently mounts a
BottomSheetModaland callsdismiss()from an effect when it should be hidden,onDismissno longer fires unless the sheet was presented first. Normal present → dismiss flows are unaffected (hadReactMountis true once presented).