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

"initialValue" initialization moved to the beginning #2

Closed
wants to merge 1 commit into from

Conversation

tobich
Copy link

@tobich tobich commented May 30, 2017

In some situation, events (such as "focus"), fired before the initialValue is used, can have unexpected side-effects on node.value. (e.g. onFocus handler may clear the input field). Moving the initialization to the top protects the original (intended) value.

@vitalyq
Copy link
Owner

vitalyq commented Jun 1, 2017

@tobich what's your use case?

The main problem here is that it won't work with controlled inputs. If onFocus handler will mutate the state, the input's value will be reverted to the new value after the change event. Please try the fork with this exmaple. Also the fix addresses only one special case while the side effects could be arbitrary, e.g., setTimeout(() => node.value = 'focused')

If all you need is the initial input's value, you could cache it before calling reactTriggerChange.

@tobich
Copy link
Author

tobich commented Jun 1, 2017

I understand. I also found workaround on our part, I am setting now focus to the input element before I call the reactTriggerChange() function. (Our usecase: onFocus clears the input field, so when I try to set new value and then call the reactTriggerChange(), the internal trigger of the "focus" event resets the original value and it get lost.) So from my point of view, this PR can be closed.

Thank you for this awesome library, I found it very useful!

@vitalyq
Copy link
Owner

vitalyq commented Jun 2, 2017

Thank you for pointing out the problem.

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.

2 participants