Prevent setState from being called on unmounted component

[sghiassy]

Thanks Jarred for the PR. I can't look at it today, but will tomorrow. Cheers

[sghiassy]

... and a year later I finally get to this PR ;)

What was the motivation for this PR? Is it performance oriented?

My main question is if we already do the check if (this.state.visibility == visibility), then aren't we already exiting early appropriately? Are we calling setVisibility when the component is unmounted?

[sghiassy]

I investigated this PR further, and I'm further confused. I tried the following code

  setVisibility(visibility) {
    if (this.rendered) {
      ... stuff
    } else {
      console.log('something"');
    }
  },

and basically something was never logged, so I'm not sure about this PR

[jarredwitt]

Yeah, I'm looking into it right now. It's been a while since I've last looked at it. I'm thinking that it had something to do with redux maybe.

[jarredwitt]

Just found it. I rendered the listview using an array provided from redux state. I have a search bar in my app that allows a user to type in a name and the list gets filtered using the typed in name. If I scrolled down the list enough to make sure that some of the cells would have their visibility set to false and then typed in a name to filter, I would start getting the warning about setting the state on a component that was no longer mounted.

My guess is that since I was filtering on an immutable data structure, my rowHasChanged would re-render the cell since the filter was active and the initial datasource would have been reduced. I'm also guessing that it has something to do with ref to the cell that's captured when the listview is first rendered using the below code:

renderRow(rowData, sectionID, rowID) {
    // Get the user's view
    var view = this.props.renderRow(rowData, sectionID, rowID);

    // Wrap the user's view in a SGListViewCell for tracking & performance
    return <SGListViewCell
              usersView={view}
              ref={(row) => {
                // Capture a reference to the cell on creation
                // We have to do it this way for ListView: https://github.com/facebook/react-native/issues/897
                PrivateMethods.captureReferenceFor(this.cellData, sectionID, rowID, row);
              }}/>
},

If the datasource for the listview changes and rowHasChanged has to re-render the cell then my guess would be that a "zombie" ref is still out there and when onChangeVisibleRows is called the "zombie" ref is being called to set the visible state to false. From what I can tell, with my fix since I was setting rendered to false in componentDidUnmount the setState call for the visibility would never fire because RN unmounted the cell setting rendered to false.

That's about as best as I can describe it. I'm still looking into it, could be an edge case.

[sibelius]

@jarredwitt could u provide a repo that reproduces this error?