idibidiart / react-native-responsive-grid

Bringing the Web's Responsive Design to React Native
Other
379 stars 42 forks source link

Row.js update a mounted or mounting component #4

Closed luosong closed 7 years ago

luosong commented 7 years ago

I use FlatList render many items, FlatList wrapped by Row.

<Row fullWidth fullHeight style={{ backgroundColor: "#f3f3f3", padding: 20 }}>
                <FlatList
                    data={this.dataItems}
                    keyExtractor={(item, index) => item.id + ''}
                    renderItem={this.renderListItem}
                />
</Row>

When pop this screen will get a warning:

Warning: Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op.

Please check the code for the Row component.

I check Row.js find warning code 78 line

this.setState({layoutTriggered: +new Date()})

idibidiart commented 7 years ago

Hi @luosong

I believe I fixed it for FlatList

See this example:

code: https://github.com/idibidiart/react-native-responsive-grid#example-4 video: https://www.youtube.com/watch?v=qLqxat3wX_8

Thank you for letting me know.

Please let me know if the issue persists on your end.

The latest is v0.28.0, so please update to that and let me know. I'll close this in the next couple of days if I don't hear back.

Thanks again.

idibidiart commented 7 years ago

Closing.

danhardman commented 6 years ago

I've just received this error on v0.34.2. The Row component is a child (albeit nested a bit) of a KeyboardAwareScrollView component from https://github.com/APSL/react-native-keyboard-aware-scroll-view.

Can replicate the issue by scrolling a bunch

idibidiart commented 6 years ago

This is a different kind of error than the one for this resolved issue. So I'll keep this one closed.

Could you please share the JSX snippet including the parent Row (and your scrollview down to the first Row descendant) ?

idibidiart commented 6 years ago

So I looked at Row.js and there is no way for the error to happen unless the direct descendant (child) of a Row component is another Row component.

Maybe what happens is that the keyboard-aware scrollview component is cloning its children and returning them (to the parent row) ? I haven't looked at the code for it.

I will start another issue to produce warnings instead of an error state when rows are children of other rows or columns are children of other columns. This way you'll get a warning in DEV mode but it should work fine as long as you realize that you're doing something that you should not be doing in general...

idibidiart commented 6 years ago

12

danhardman commented 6 years ago

I'll look into keyboard-aware-scroll-view to see if that is the case, but I'm not sure how it could happen. The current JSX is:

<KeyboardAwareScrollView>
  <View>
    <Row>
      { this.props.foos.map((bar, index) => (
        <Col key={index}>
           //
        </Col> ))
      }
    </Row>
  </View>
</KeyboardAwareScrollView>
idibidiart commented 6 years ago

Sorry, I misunderstood your report.

You're saying that you're seeing this warning:

Warning: Can only update a mounted or mounting component. This usually means you called setState, replaceState, or forceUpdate on an unmounted component. This is a no-op.

I thought you were getting an error generated by the grid due to a Row in Row or Col in Col situation, which is why I didn't re-open this issue.

I've made a breaking change that will fix this and simplify the grid's code (as well as giving us better performance by eliminating unnecessary re-rendering) and v0.35.0 should fix this for you. What was happening is that the Row was growing in height since it had to no fixed height specified and it was invoking onLayout due to changes in height as more cols were added, which re-renders the row. One way to solve it would have been to keep track of all the re-render requests then cancel all of them in a synchronous loop but that's just a sign of a bad design decision, so instead I've opted to make a breaking change and require use of Grid component at the top of the view hierarchy for any responsive effect to do with orientation changes. To react to changes that don't affect the Grid's frame we would use Flexbox constraints to re-layout without having to manage that from JS. I'll have some example to demonstrate that in the next minor update.

0.35.0 is now up on NPM.

@danhardman

danhardman commented 6 years ago

Appreciate it! My fault on the initial report, I should have been clearer. I will look to implement the new changes as soon as possible to give feedback :)

Really appreciate the quick responses