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

usafe component will receive props #538

Merged
merged 7 commits into from
Jan 19, 2020

Conversation

benitogf
Copy link
Contributor

@MaxSvargal
Copy link

Let's refactor it to static getDerivedStateFromProps

@benitogf
Copy link
Contributor Author

Hello @MaxSvargal made some progress on using static getDerivedStateFromProps however got blocked on https://github.com/oliviertassinari/react-swipeable-views/pull/538/files#diff-763c63c1eb389e9b13f4cc3e2eff08abR354 as I mentioned in the comment this method relies on having access to the class this which is not available from getDerivedStateFromProps any suggestions?

}

// this.setIndexCurrent(index);
// this method heavily realies on having access to the class :/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

realies -> relies

@vanhoutenbos
Copy link
Collaborator

@benitogf Can you checkout the unit test that failed, it returned the wrong transform. could you also fix the typo in the comments?

@benitogf
Copy link
Contributor Author

benitogf commented Dec 5, 2019

@jeanpoelie will take a look to this one on the weekend 👍 could we merge the UNSAFE_componentWillReceiveProps while we switch to getDerivedStateFromProps though?

@vanhoutenbos
Copy link
Collaborator

I went ahead and merged #551 after resolving the conflicts.

@vanhoutenbos
Copy link
Collaborator

@benitogf did you find the time to take a look at this?

@benitogf
Copy link
Contributor Author

Hello, I was trying to take a look at this one but found that currently the class has both unsafe labeled and regular methods is there a reason for that? shouldn't just the unsafe call be ok?

about the issue itself, found that this.indexCurrent = indexCurrent allows switching between a "controled" and "uncontrolled" state, moving forward I think that the component should be fully controled after the initial render, changing the state acording to prop changes is an antipattern? Just sharing what I have found so far, glad to get other ideas about it

@vanhoutenbos vanhoutenbos merged commit c33feb9 into oliviertassinari:master Jan 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants