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

Clear input value on receiving props with another value. #2183

Merged
merged 2 commits into from
Dec 13, 2017
Merged

Clear input value on receiving props with another value. #2183

merged 2 commits into from
Dec 13, 2017

Conversation

yuri-sakharov
Copy link
Contributor

Hi,
Should be fixed this bug #2155.
Thanks.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.02%) to 93.45% when pulling d212323 on yuri-sakharov:bug/clear-input-value into 75f3043 on JedWatson:master.

@JedWatson
Copy link
Owner

Solid fix @yuri-sakharov, thanks!

@JedWatson JedWatson merged commit 700b79a into JedWatson:master Dec 13, 2017
@yuri-sakharov yuri-sakharov deleted the bug/clear-input-value branch December 14, 2017 07:35
@yuri-sakharov
Copy link
Contributor Author

@JedWatson thank you too.

@jharris4
Copy link
Contributor

FYI this was a breaking change for me (and took me a little while to track down after upgrading!)

My value prop (actually a list of values since I'm in multi mode) are coming from an immutable collection which I was converting to an array of values in a render function.

I had to refactor things to instead do the conversion in componentWillReceiveProps (only if the immutable collection changed) and assign to state for use in the render function.

I can see the value of this PR, but it might be helpful to draw attention to it in the docs or something in case other people get tripped up by this as well.

@yuri-sakharov
Copy link
Contributor Author

Hi, @jharris4
Thank you for comment I fixed it in #2299

@hiredgun
Copy link

hiredgun commented Jun 12, 2018

@yuri-sakharov why wasn't this fix merged?

@yuri-sakharov
Copy link
Contributor Author

@hiredgun I don't know and don't have rights for merge. The question should be addressed to collaborators.

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

Successfully merging this pull request may close these issues.

5 participants