Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

can't perform a react state updated on an unmounted component #69

Closed
aboveyunhai opened this issue Jun 18, 2020 · 18 comments
Closed

can't perform a react state updated on an unmounted component #69

aboveyunhai opened this issue Jun 18, 2020 · 18 comments

Comments

@aboveyunhai
Copy link

aboveyunhai commented Jun 18, 2020

@SteffeyDev I was trying to create a list of items along with tooltips,
While closing one of the tooltips, I got the warning can't perform a react state updated on an unmounted component.

It seems like the problem has nothing to do with my list example, the problem always appears in react native bare flow when popover is closed.

@aboveyunhai aboveyunhai changed the title can't perform a react state updated on an unmounted component when can't perform a react state updated on an unmounted component Jun 18, 2020
@SteffeyDev
Copy link
Owner

I put your code into a snack and am unable to reproduce the warning: https://snack.expo.io/@steffeydev/6f4b71

Can you please create a snack or a full working example that shows the warning?

I will note that this is not a bug, just a small potential memory leak that probably has no impact. Of course, if I can reproduce it I will fix it nonetheless.

@aboveyunhai
Copy link
Author

After 2 hours debugging with expo and mixing up with all possible combination I can come up from my test app. It seems like the problem does not exist.
So I create a fresh new react native app (bare flow without expo) and run it on both my phone and emulator, the problem appears as long as I closed the popover.
image

Just some information about my emulator;

image

My phone is samsunng S9, Android Version is 10

@rafaelsaback
Copy link

I'm facing the same behavior. The first time I run the application when I close the popover I receive the same warning message.

@SteffeyDev
Copy link
Owner

Alright, I'm plugging it into my ejected app now so hopefully can reproduce and fix.

@rafaelsaback
Copy link

@SteffeyDev, if it serves of any help, this is the stack trace that the warning returns to me:

image

@rafaelsaback
Copy link

rafaelsaback commented Jun 20, 2020

@SteffeyDev, my best guess is that this is being caused by the call to setState within onCloseStart (AdaptivePopover), here:

https://github.com/SteffeyDev/react-native-popover-view/blob/master/src/Popover.tsx#L495

Since onCloseStart is called from componentWillUnmount (BasePopover), this actually causes setState to be called when the component is about to be unmounted.

@SteffeyDev
Copy link
Owner

Yep, I found that as well and fixed in 3.0.2. Let me know if it works for you as well!

@rafaelsaback
Copy link

Sorry for the late reply. Unfortunately, the issue is still there - I've tested version 3.0.3. Apparently, when I dismiss the popover, onCloseStart is called twice. On the first time that it's called, isMounted is true which causes the react state update issue - for what it's worth, on the second time isMounted is false.

@SteffeyDev
Copy link
Owner

@rafaelsaback Can you pass debug={true} and paste the logs here from when onCloseStart is called twice? Or else tell me how to reproduce so I can debug and produce the logs myself? Looking at the code I can't figure out what would cause the issue.

@aboveyunhai
Copy link
Author

@rafaelsaback Can you pass debug={true} and paste the logs here from when onCloseStart is called twice? Or else tell me how to reproduce so I can debug and produce the logs myself? Looking at the code I can't figure out what would cause the issue.

I just check the new version in both brand new create-react-native-app and my own toy app which had the problem in the past. The problem is not longer exist. So I have to assume @rafaelsaback 's problem might exist somewhere else like where the parent of popover is unmounted. Other than, I believe my problem is fixed. But the problem can be some edge cases I did not encounter.

@rafaelsaback
Copy link

rafaelsaback commented Jul 9, 2020

Hey @SteffeyDev, I ran it once again to generate the debug logs, but funnily enough, I didn't face the issue any longer. I even tried different phones, but the warning was no longer displayed. Thanks for fixing this and feel free to close the issue. 👍

@SteffeyDev
Copy link
Owner

Glad to hear!

@onlyling
Copy link

@rafaelsaback Can you pass debug={true} and paste the logs here from when onCloseStart is called twice? Or else tell me how to reproduce so I can debug and produce the logs myself? Looking at the code I can't figure out what would cause the issue.

There is a warning message only when opening and closing for the first time

{
  "react": "17.0.2",
  "react-native": "0.66.4",
  "react-native-popover-view": "^5.0.1",
}

Code

import React, { useRef, useState } from 'react'

import { Text, TouchableOpacity } from 'react-native'

const BasicPopover: React.FC = () => {
  const touchable = useRef<TouchableOpacity>(null)
  const [showPopover, setShowPopover] = useState(false)

  return <>
          <TouchableOpacity
            ref={touchable}
            onPress={() => {
              setShowPopover(true)
            }}>
            <Text>target</Text>
          </TouchableOpacity>
          <RNPopoverView
            debug
            from={touchable}
            isVisible={showPopover}
            onRequestClose={() => {
              setShowPopover(false)
            }}>
            <Text>content</Text>
          </RNPopoverView>
        </>
}

Log

 LOG  [2022-04-11T06:49:04.599Z] calculateRectFromRef - waiting for ref
 LOG  [2022-04-11T06:49:04.599Z] calculateRectFromRef - waiting for ref to move
 LOG  [2022-04-11T06:49:04.601Z] calculateRectFromRef - calculated Rect: {"x":0,"y":91,"width":390,"height":17}
 LOG  [2022-04-11T06:49:04.607Z] setDefaultDisplayArea - newDisplayArea: {"x":0,"y":0,"width":390,"height":844}
 LOG  [2022-04-11T06:49:04.608Z] setDefaultDisplayArea - displayAreaOffset: {"x":0,"y":0}
 LOG  [2022-04-11T06:49:04.609Z] calculateRectFromRef - waiting for ref
 LOG  [2022-04-11T06:49:04.609Z] calculateRectFromRef - waiting for ref to move
 LOG  [2022-04-11T06:49:04.622Z] measureContent - new requestedContentSize: {"width":49.66666793823242,"height":17} (used to be null)
 LOG  [2022-04-11T06:49:04.623Z] handleChange - waiting 100ms to accumulate all changes
 LOG  [2022-04-11T06:49:04.740Z] handleChange - requestedContentSize: {"width":49.66666793823242,"height":17}
 LOG  [2022-04-11T06:49:04.741Z] handleChange - displayArea: {"x":0,"y":0,"width":390,"height":844}
 LOG  [2022-04-11T06:49:04.741Z] handleChange - fromRect: {"x":0,"y":91,"width":390,"height":17}
 LOG  [2022-04-11T06:49:04.741Z] handleChange - placement: "auto"
 LOG  [2022-04-11T06:49:04.742Z] computeAutoGeometry - displayArea: {"x":0,"y":0,"width":390,"height":844}
 LOG  [2022-04-11T06:49:04.742Z] computeAutoGeometry - fromRect: {"x":0,"y":91,"width":390,"height":17}
 LOG  [2022-04-11T06:49:04.742Z] computeAutoGeometry - List of availabe space: {"left":{"sizeAvailable":-16,"sizeRequested":49.66666793823242},"right":{"sizeAvailable":-16,"sizeRequested":49.66666793823242},"top":{"sizeAvailable":75,"sizeRequested":17},"bottom":{"sizeAvailable":720,"sizeRequested":17}}
 LOG  [2022-04-11T06:49:04.742Z] computeAutoGeometry - Found best postition for placement: "bottom"
 LOG  [2022-04-11T06:49:04.743Z] computeGeometry - initial chosen geometry: {"popoverOrigin":{"x":170.1666660308838,"y":108},"anchorPoint":{"x":195,"y":108},"placement":"bottom","forcedContentSize":{"width":370,"height":726},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-04-11T06:49:04.743Z] computeGeometry - final chosen geometry: {"popoverOrigin":{"x":170.1666660308838,"y":108},"anchorPoint":{"x":195,"y":108},"placement":"bottom","forcedContentSize":{"width":370,"height":726},"viewLargerThanDisplayArea":{"height":false,"width":false}}
 LOG  [2022-04-11T06:49:04.747Z] handleChange - animating in
 LOG  [2022-04-11T06:49:04.747Z] getTranslateOrigin - popoverSize: {"width":49.66666793823242,"height":25}
 LOG  [2022-04-11T06:49:04.747Z] getTranslateOrigin - anchorPoint: {"x":195,"y":108}
 LOG  [2022-04-11T06:49:04.748Z] animateIn - translateStart: {"x":170.1666660308838,"y":1783.5}
 LOG  [2022-04-11T06:49:04.749Z] animateIn - translatePoint: {"x":170.1666660308838,"y":108}
 LOG  [2022-04-11T06:49:04.753Z] Setting up keyboard listeners
 LOG  [2022-04-11T06:49:04.772Z] measureContent - Skipping, content size did not change
 LOG  [2022-04-11T06:49:05.085Z] animateIn - onOpenComplete - Calculated Popover Rect: {"x":170.1666717529297,"y":116,"width":49.66666793823242,"height":17}
 LOG  [2022-04-11T06:49:05.086Z] animateIn - onOpenComplete - Calculated Arrow Rect: {"x":187.1666717529297,"y":108,"width":18,"height":10}
 LOG  [2022-04-11T06:49:09.237Z] [BasePopover] componentDidUpdate - changedProps: ["isVisible"]
 LOG  [2022-04-11T06:49:09.237Z] componentDidUpdate - isVisible changed, now false
 LOG  [2022-04-11T06:49:09.238Z] animateOut - isMounted: true
 LOG  [2022-04-11T06:49:09.238Z] getTranslateOrigin - popoverSize: {"width":49.66666793823242,"height":25}
 LOG  [2022-04-11T06:49:09.238Z] getTranslateOrigin - anchorPoint: {"x":195,"y":108}
 LOG  [2022-04-11T06:49:09.239Z] componentDidUpdate - Hiding popover
 LOG  [2022-04-11T06:49:09.243Z] Tearing down keyboard listeners
 ERROR  Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
    in AdaptivePopover (created by RNModalPopover)
    in RCTView (at View.js:32)
    in View (at AppContainer.js:92)
    in RCTView (at View.js:32)
    in View (at AppContainer.js:119)
    in AppContainer (at Modal.js:242)
    in RCTView (at View.js:32)
    in View (at Modal.js:270)
    in VirtualizedListContextResetter (at Modal.js:268)
    in RCTModalHostView (at Modal.js:248)

@jonathanj
Copy link

jonathanj commented Apr 24, 2022

I see this issue is closed @SteffeyDev, however this is definitely still happening for me, in 5.0.0 (RN 0.67.3 on iOS 15.4), when closing the popover. The source of the issue reported by React Native is calling setState in onCloseComplete from AdaptivePopover.

My conclusion is that the issue is because AdaptivePopover calls onCloseComplete synchronously and sets state afterwards, it becomes possible for onCloseComplete to cause AdaptivePopover to unmount, and then setState is called on an unmounted component.

I added some debugging to the relevant parts of the code to trace the execution of dismissing the popover:

 LOG  [2022-04-24T11:48:08.140Z] animateIn - onOpenComplete - Calculated Popover Rect: {"x":154.75,"y":186,"width":104.5,"height":359.5}
 LOG  [2022-04-24T11:48:08.141Z] animateIn - onOpenComplete - Calculated Arrow Rect: {"x":199.25,"y":178,"width":18,"height":10}
 LOG  🐞:Popover(RNModal):onRequestClose before onRequestClose
 LOG  🐞:user code:onRequestClose
 LOG  🐞:Popover(RNModal):onRequestClose after onRequestClose
 LOG  🐞:Popover(RNModal):render {"isVisible": false}
 LOG  🐞:AdaptivePopover:render {"fromRect": {"height": 48, "width": 414, "x": 0, "y": 130}, "fromRef": {"current": {"_children": [Array], "_internalFiberInstanceHandleDEV": [FiberNode], "_nativeTag": 2445, "viewConfig": [Object]}}, "showing": true}
 LOG  [2022-04-24T11:48:08.937Z] [BasePopover] componentDidUpdate - changedProps: ["isVisible"]
 LOG  [2022-04-24T11:48:08.938Z] componentDidUpdate - isVisible changed, now false
 LOG  [2022-04-24T11:48:08.938Z] animateOut - isMounted: true
 LOG  [2022-04-24T11:48:08.938Z] getTranslateOrigin - popoverSize: {"width":104.5,"height":367.5}
 LOG  [2022-04-24T11:48:08.938Z] getTranslateOrigin - anchorPoint: {"x":207,"y":178}
 LOG  [2022-04-24T11:48:08.938Z] componentDidUpdate - Hiding popover
 LOG  [2022-04-24T11:48:08.942Z] Tearing down keyboard listeners
 LOG  🐞:AdaptivePopover:render {"fromRect": {"height": 48, "width": 414, "x": 0, "y": 130}, "fromRef": {"current": {"_children": [Array], "_internalFiberInstanceHandleDEV": [FiberNode], "_nativeTag": 2445, "viewConfig": [Object]}}, "showing": true}
 LOG  🐞:BasePopover:animateTo starting
 LOG  🐞:BasePopover:animateTo timeout running {"isMounted": true}
 LOG  🐞:AdaptivePopover:onCloseComplete before props.onCloseComplete {"isMounted": true}  ──┐
 LOG  🐞:AdaptivePopover:componentWillUnmount                                                ├─ 🚨 HERE
 LOG  🐞:BasePopover:componentWillUnmount                                                    │
 LOG  🐞:AdaptivePopover:onCloseComplete before setting showing=false {"isMounted": false} ──┘
 ERROR  Warning: Can't perform a React state update on an unmounted component. This is a no-op, but it indicates a memory leak in your application. To fix, cancel all subscriptions and asynchronous tasks in the componentWillUnmount method.
    in AdaptivePopover (created by RNModalPopover)
    in RCTView (at View.js:32)
    in View (at AppContainer.js:92)
    in RCTView (at View.js:32)
    in View (at AppContainer.js:119)
    in AppContainer (at Modal.js:244)
    in RCTView (at View.js:32)
    in View (at Modal.js:273)
    in VirtualizedListContextResetter (at Modal.js:271)
    in RCTModalHostView (at Modal.js:250)
 LOG  🐞:user code:onCloseComplete
 LOG  🐞:Popover(RNModal):render {"isVisible": false}
 LOG  🐞:Popover(RNModal):render {"isVisible": false}
 LOG  🐞:Popover(RNModal):onRequestClose after setState

In the log you can see that 🐞:AdaptivePopover:onCloseComplete is emitted once before calling props.onCloseComplete and is still mounted at that point, however after onCloseComplete is called it has been unmounted right before calling setState({showing: false}).

Something that worked for me was to use the callback argument of setState to call onCloseComplete, changing this code to:

onCloseComplete={() => {
  this.setState({ showing: false }, () => {
    if (onCloseComplete) onCloseComplete();
  });
}}

Which makes sense because once the state has been updated to showing: false then the close is truly complete, but I'll admit to not fully exploring the implications.

@SteffeyDev SteffeyDev reopened this Apr 24, 2022
@SteffeyDev
Copy link
Owner

Thanks for the research, I'll look into this again next time I'm working this project.

jeraldgan added a commit to jeraldgan/react-native-popover-view that referenced this issue May 1, 2022
@mazenchami
Copy link

@SteffeyDev just opened a PR to solve this issue.

@mazenchami
Copy link

Thank you @jonathanj for the deep dive and solution.

@SteffeyDev
Copy link
Owner

@mazenchami's PR included in 5.0.2. Going to close for now, will re-open if you still encounter the issue on 5.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants