colorfy-software / react-native-modalfy

🥞 Modal citizen of React Native.
https://colorfy-software.gitbook.io/react-native-modalfy
MIT License
1.06k stars 42 forks source link

closeAllModals() and others sometimes do not run #81

Closed okcompewter closed 2 years ago

okcompewter commented 2 years ago

I've updated to the latest 3.3.0 release. But now I'm seeing another issue.

In my app, when I navigate the user to another screen, I like to close all open modals (sometimes the user is navigated due to a change in data, for example where they are being attacked by another player, so I need to react to it).

I'm seeing intermittent behavior where closeAllModals() is not being run at all though. I've passed in a logging callback, and usually I don't see that logged, and the modal stays open on top of the navigation change below the modal, so the user doesn't notice they are being attacked.

I even log currentModal immediately before to check if there is a currentModal open, and it is correct. Any ideas?

  const navToAttack = async () => {
    props.navigation.navigate("attack", {
      attackId: user.attackId
    })
    console.log(currentModal)
    closeAllModals(() => {
      console.log('closed all')
    })
  }

When the modal stays open occasionally, I just see the currentModal logged, but no callback logging: MainMenuModal

Where in the code could I manually do some logging in your plugin to help troubleshoot? I tried adding logging to both files' functions and rebuilding, but I never see the added statement logged: node_modules\react-native-modalfy\lib\commonjs\lib\ModalState.js node_modules\react-native-modalfy\lib\module\lib\ModalState.js

  const closeAllModals = () => {
    console.log('test closeAllModals')
    const {
      openedItems
    } = state.stack;
    openedItems.clear();
    setState(currentState => ({
      currentModal: null,
      stack: { ...currentState.stack,
        openedItems
      }
    }));
  };

Thanks for any help in troubleshooting this issue!

CharlesMangwa commented 2 years ago

Hi @okcompewter! I'll have a look at this, if you could join a reproducible demo on Snack in the meantime, that'd be very helpful.

As well, you can add your logs in here: node_modules/react-native-modalfy/src/lib as Metro uses the library source files directly.

okcompewter commented 2 years ago

Thanks for that - I dropped a log at the start of closeAllModals() and did some testing:

const closeAllModals = () => {
    console.log('CLOSING ALL MODALS')

    const { openedItems } = state.stack

    openedItems.clear()

    setState(currentState => ({
      currentModal: null,
      stack: { ...currentState.stack, openedItems },
    }))
  }

It seems that the both the log and the callback log never appear when there are no modals open (by design?), including when it doesn't "think" any are open.

It's so intermittent, I can't get a good repro on it.

okcompewter commented 2 years ago

I logged the stack inside , and it seems every time it doesn't work, one stack logs with accumulating pendingClosingActionsSize (scroll to right), and one that is cleared out:

 LOG  state {"currentModal": "BaseFloorplanModal", "stack": {"content": [[Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object]], "defaultOptions": {"animateInConfig": [Object], "animateOutConfig": [Object], "backBehavior": "pop", "backdropAnimationDuration": 300, "backdropOpacity": 0.95, "containerStyle": [Object], "disableFlingGesture": true, "position": "center", "transitionOptions": [Function transitionOptions]}, "names": ["MainMenuModal", "InventoryModal", "PlayerProfileModal", "BaseCampfireModal", "BaseBedModal", "BaseTanneryModal", "BaseOutfitterModal", "BaseMailboxModal", "BaseHarvesterModal", "BaseWorkbenchModal", "BaseFurnaceModal", "BaseAnvilModal", "BaseTripwireModal", "BaseCrucibleModal", "BaseStableModal", "BaseChemModal", "BaseUpgradeModal", "BaseRaidedModal", "BaseStationModal", "BaseFloorplanModal", "AttackWinModal", "AttackRunModal", "WorldResultsModal", "WorldLeaderboardModal", "ChatModal", "ShopModal"], "openedItems": Set {[Object]}, "openedItemsSize": 1, "pendingClosingActions": Set {[Object], [Object], [Object], [Object]}, "pendingClosingActionsSize": 4}}

... close manually and trigger again ...

 LOG  state {"currentModal": "BaseFloorplanModal", "stack": {"content": [[Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object]], "defaultOptions": {"animateInConfig": [Object], "animateOutConfig": [Object], "backBehavior": "pop", "backdropAnimationDuration": 300, "backdropOpacity": 0.95, "containerStyle": [Object], "disableFlingGesture": true, "position": "center", "transitionOptions": [Function transitionOptions]}, "names": ["MainMenuModal", "InventoryModal", "PlayerProfileModal", "BaseCampfireModal", "BaseBedModal", "BaseTanneryModal", "BaseOutfitterModal", "BaseMailboxModal", "BaseHarvesterModal", "BaseWorkbenchModal", "BaseFurnaceModal", "BaseAnvilModal", "BaseTripwireModal", "BaseCrucibleModal", "BaseStableModal", "BaseChemModal", "BaseUpgradeModal", "BaseRaidedModal", "BaseStationModal", "BaseFloorplanModal", "AttackWinModal", "AttackRunModal", "WorldResultsModal", "WorldLeaderboardModal", "ChatModal", "ShopModal"], "openedItems": Set {[Object]}, "openedItemsSize": 1, "pendingClosingActions": Set {[Object], [Object], [Object], [Object], [Object]}, "pendingClosingActionsSize": 5}}
 LOG  state {"currentModal": "BaseFloorplanModal", "stack": {"content": [[Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object]], "defaultOptions": {"animateInConfig": [Object], "animateOutConfig": [Object], "backBehavior": "pop", "backdropAnimationDuration": 300, "backdropOpacity": 0.95, "containerStyle": [Object], "disableFlingGesture": true, "position": "center", "transitionOptions": [Function transitionOptions]}, "names": ["MainMenuModal", "InventoryModal", "PlayerProfileModal", "BaseCampfireModal", "BaseBedModal", "BaseTanneryModal", "BaseOutfitterModal", "BaseMailboxModal", "BaseHarvesterModal", "BaseWorkbenchModal", "BaseFurnaceModal", "BaseAnvilModal", "BaseTripwireModal", "BaseCrucibleModal", "BaseStableModal", "BaseChemModal", "BaseUpgradeModal", "BaseRaidedModal", "BaseStationModal", "BaseFloorplanModal", "AttackWinModal", "AttackRunModal", "WorldResultsModal", "WorldLeaderboardModal", "ChatModal", "ShopModal"], "openedItems": Set {[Object]}, "openedItemsSize": 1, "pendingClosingActions": Set {}, "pendingClosingActionsSize": 0}}

... close manually and trigger again ...

 LOG  state {"currentModal": "BaseFloorplanModal", "stack": {"content": [[Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object]], "defaultOptions": {"animateInConfig": [Object], "animateOutConfig": [Object], "backBehavior": "pop", "backdropAnimationDuration": 300, "backdropOpacity": 0.95, "containerStyle": [Object], "disableFlingGesture": true, "position": "center", "transitionOptions": [Function transitionOptions]}, "names": ["MainMenuModal", "InventoryModal", "PlayerProfileModal", "BaseCampfireModal", "BaseBedModal", "BaseTanneryModal", "BaseOutfitterModal", "BaseMailboxModal", "BaseHarvesterModal", "BaseWorkbenchModal", "BaseFurnaceModal", "BaseAnvilModal", "BaseTripwireModal", "BaseCrucibleModal", "BaseStableModal", "BaseChemModal", "BaseUpgradeModal", "BaseRaidedModal", "BaseStationModal", "BaseFloorplanModal", "AttackWinModal", "AttackRunModal", "WorldResultsModal", "WorldLeaderboardModal", "ChatModal", "ShopModal"], "openedItems": Set {[Object]}, "openedItemsSize": 1, "pendingClosingActions": Set {}, "pendingClosingActionsSize": 0}}
 LOG  state {"currentModal": "BaseFloorplanModal", "stack": {"content": [[Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object], [Object]], "defaultOptions": {"animateInConfig": [Object], "animateOutConfig": [Object], "backBehavior": "pop", "backdropAnimationDuration": 300, "backdropOpacity": 0.95, "containerStyle": [Object], "disableFlingGesture": true, "position": "center", "transitionOptions": [Function transitionOptions]}, "names": ["MainMenuModal", "InventoryModal", "PlayerProfileModal", "BaseCampfireModal", "BaseBedModal", "BaseTanneryModal", "BaseOutfitterModal", "BaseMailboxModal", "BaseHarvesterModal", "BaseWorkbenchModal", "BaseFurnaceModal", "BaseAnvilModal", "BaseTripwireModal", "BaseCrucibleModal", "BaseStableModal", "BaseChemModal", "BaseUpgradeModal", "BaseRaidedModal", "BaseStationModal", "BaseFloorplanModal", "AttackWinModal", "AttackRunModal", "WorldResultsModal", "WorldLeaderboardModal", "ChatModal", "ShopModal"], "openedItems": Set {[Object]}, "openedItemsSize": 1, "pendingClosingActions": Set {[Object], [Object], [Object], [Object], [Object], [Object]}, "pendingClosingActionsSize": 6}}

May be a sign of something else going on with my ModalProvider/Stack from before... any ideas why this would be happening on more than one stack?

okcompewter commented 2 years ago

Some more debugging: When logging here at the end of queueClosingAction:

    console.log('pendingClosingActions', pendingClosingActions)

    return [...pendingClosingActions].slice(-1)[0]
  }

and here at closeAllModals:

const closeAllModals = () => {
    console.log('CLOSING ALL MODALS')

and here from the caller:

  const navToAttack = async () => {
    props.navigation.navigate("attack", {
      attackId: user.attackId
    })
    console.log('attempting closing all modals')
    console.log('currentModal:', currentModal)
    closeAllModals(() => {
      console.log('closed all modals callback')
    })
  }

when it works, it logs this:

 LOG  attempting closing all modals
 LOG  currentModal: BaseFloorplanModal
 LOG  pendingClosingActions Set {{"action": "closeAllModals", "callback": [Function anonymous], "currentModalHash": "BaseFloorplanModal_90wr9yudw", "hash": "closeAllModals_3mnvj7nge", "modalName": undefined}}
 LOG  CLOSING ALL MODALS
 LOG  closed all modals callback

when it doesn't work, it logs this (notice the accumulating objects in Set, one of which has some undefined values):

 LOG  attempting closing all modals
 LOG  currentModal: BaseFloorplanModal
 LOG  pendingClosingActions Set {{"action": "closeAllModals", "callback": undefined, "currentModalHash": undefined, "hash": "closeAllModals_12tl9jiig", "modalName": undefined}, {"action": "closeAllModals", "callback": [Function anonymous], "currentModalHash": "BaseFloorplanModal_ku5502evn", "hash": "closeAllModals_bi0sb6bfr", "modalName": undefined}}
okcompewter commented 2 years ago

I think it's coming down to currentModalHash sometimes being undefined, which could be possible given this code change:

https://github.com/colorfy-software/react-native-modalfy/commit/28cf12b267eea773767952d8f8220a7f6c1e950f#diff-5af16b92751db0283d500b403f3d01978d846f73e43899b315f5abc730de8000R263

currentModalHash: [...currentState.stack.openedItems].slice(-1)[0]?.hash,

The question is how does it get to be undefined in the first place, and how to still remove the modal properly even if it is? Otherwise, they just queue up in pendingClosingActions and are never cleared out.

okcompewter commented 2 years ago

Looks like sometimes currentState.stack.openedItems is empty, even when there is actually still a modal open:

const { pendingClosingActions } = setState(currentState => {
      console.log('currentState.stack.openedItems', currentState.stack.openedItems)
      return ({
      ...currentState,
      stack: {
        ...currentState.stack,
        pendingClosingActions: currentState.stack.pendingClosingActions.add({
          hash,
          action,
          callback,
          modalName,
          currentModalHash: [...currentState.stack.openedItems].slice(-1)[0]?.hash,
        }),
      },
    })}).stack

When this is the case, no closing of the modal happens, and the item sticks around forever in pendingClosingActions (is never cleared) with an undefined value for currentModalHash.

Next question is why is currentState.stack.openedItems sometimes empty, when it shouldn't be?

okcompewter commented 2 years ago

I think I may have found something: I had a few screens that would also closeAllModals(), or the modal would close itself with closeModal() around the same time others may be also calling closeAllModals().

Might be some state race condition, where the open modal is no longer in openedItems when closeAllModals() runs, and if that's the case, it just gets stuck as a pending action, and things go worse from there.

Still testing out things...

okcompewter commented 2 years ago

Hmm, still happening occasionally, even after reducing several "close" methods happening near each other.

Basically whenever the state values get to this, closeAllModals() will never work again:

...
"openedItems": Set {}, "openedItemsSize": 0, "pendingClosingActions": Set {{"action": "closeAllModals", "callback": undefined, "currentModalHash": undefined, "hash": "closeAllModals_k0bbl4954", "modalName": undefined}}, "pendingClosingActionsSize": 1}
...

Is it possible the item is removed from openedItems before the actual pendingCloseAction is dealth with?

okcompewter commented 2 years ago

Ok, I've verified that the issue was happening when closeAllModals() was being called when there are no open items.

This needs handled better, so that closeAllModals() can be called safely from anywhere in the application, at any time. If there are no open modals, no pending action needs added.

Adding a simple check like below seems to fix the issue for me, and prevents a bad pending action from being added:

  const queueClosingAction = <P>({
    action,
    callback,
    modalName,
  }: ModalStateType<P>['queueClosingAction']['arguments']): ModalStateType<P>['queueClosingAction']['arguments'] => {
    console.log('queueClosingAction', action)
    const {
      stack: { names },
    } = state

    if (!state?.stack?.openedItems?.size) {
      console.log('ignoring, no modals to close')
      return
    }

For now, I'm working around this at the application level by always preceding closeAllModals() with a check for currentModal: currentModal && closeAllModals()

I hope that helps you with a fix, and thanks for all you do!

CharlesMangwa commented 2 years ago

Hey @okcompewter! Thanks for investigating this! I'll push a patch release with this check inside queueClosingAction(). Let me know if it fully addresses the issue for you. If not, please send me a repro on Snack to help me debug this!

otaviogaiao commented 2 years ago

@CharlesMangwa I think the issue was not resolved. I just encountered it, and I'm using the latest version (3.3.1). In my case I have a function on a button in the modal that calls closeModal and executes a navigation.replace. The modal gets stuck in the screen. In order to make it work, I had to to like this:

    const execute = (func: () => void) => { //gambiarra por causa de bug na lib
        setTimeout(func, 500);
    }

    const onPressPrimary = () => {
        closeModal();
        execute(() => {
            if (primaryAction) {
                primaryAction();
            }
        })
    }

Ugly but solves the problem without any noticeable side effect to the user.

CharlesMangwa commented 2 years ago

Hey @otaviogaiao! Did you try to invoke your callback as mentioned in the docs? If you can submit a repro on Snack, that'd be very helpful!

otaviogaiao commented 2 years ago

Hey @otaviogaiao! Did you try to invoke your callback as mentioned in the docs? If you can submit a repro on Snack, that'd be very helpful!

I tried to replicate this error in a snack, but couldn't. I will check my code again to see if im doing something wrong. I will post if I find out anything. Thanks

willadamskeane commented 1 year ago

I'm running into this issue intermittently, still trying to uncover the exact steps to replicate. But the outcome is that I'm unable to successfully call closeModals or closeAllModals, and the modal stack is filled with pendingClosingActions that don't resolve.

iyansr commented 1 year ago

facing same issue

duylinhmeliodas commented 5 months ago

I'm facing too :( May be I must to remove the lib