remove componentWillReceiveProps #394
Conversation
|
We should try to get this in |
|
Hi @zackseuberling, it looks like there's something that needs to be changed in your code. You can't just change the method name cause |
|
re-reading the original code, it seems like all of React states explicitly in their docs for
|
zackseuberling
changed the title
|
I tried to use your branch but the warning was still there. It looks like you didn't build the project before uploading. |
I was under the impression that the build would be automated. @MarkEhr, I ran a build locally, and did get it to work via |
|
Hi, Could you please review this PR... It would be nice of you @leandrowd |
|
Hi @leandrowd , could you please review and merge this? Thank you! |
| if (prevProps.selectedItem !== this.props.selectedItem) { | ||
| this.updateSizes(); | ||
| this.moveTo(this.props.selectedItem); | ||
| } | ||
|
|
||
| if (prevProps.autoPlay !== this.props.autoPlay) { | ||
| if (this.props.autoPlay) { | ||
| this.setupAutoPlay(); | ||
| } else { | ||
| this.destroyAutoPlay(); | ||
| } | ||
|
|
||
| this.setState({ autoPlay: this.props.autoPlay }); | ||
| } |
There was a problem hiding this comment.
looking at my code again, I think I may have mixed up some logic about nextProps/prevProps/prevState?
this is emphasizing for me how confusing componentWillReceiveProps is.
this is correct, right:
componentWillReceiveProps:nextProps :: componentDidUpdate:this.props
| if (prevState.swiping && !this.state.swiping) { | ||
| // We stopped swiping, ensure we are heading to the new/current slide and not stuck | ||
| this.resetPosition(); | ||
| } | ||
|
|
||
| if (prevProps.selectedItem !== this.props.selectedItem) { |
There was a problem hiding this comment.
believe the logic here should actually be (since the component has updated at this point), if this.props.selectedItem does not match this.state.selectedItem, but this is a very confusing thing to write…
| this.updateSizes(); | ||
| this.moveTo(nextProps.selectedItem); | ||
| } | ||
|
|
There was a problem hiding this comment.
if selectedItem prop is going to change, and it doesn't match this.state.selectedItem… do thing
|
Thanks for the contribution :) |
closes #355
all tests are still passing, but I'm not entirely sure what is supposed to happen with this life cycle hook, so unsure if this could introduce new bugs