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

Warning on Input Component #135

Closed
luciemac opened this issue Sep 18, 2018 · 7 comments
Closed

Warning on Input Component #135

luciemac opened this issue Sep 18, 2018 · 7 comments

Comments

@luciemac
Copy link

I am having a warning using ReactTags:

Warning: Failed prop type: You provided a `value` prop to a form field without an `onChange` handler. This will render a read-only field. If the field should be mutable use `defaultValue`. Otherwise, set either `onChange` or `readOnly`.
    in input (created by Input)
    in div (created by Input)
    in Input (created by ReactTags)
    in div (created by ReactTags)
    in div (created by ReactTags)

From what is said here we should probably change https://github.com/i-like-robots/react-tags/blob/master/lib/Input.js#L78 by something like:
defaultValue={query}

What do you think?
Thanks!

@gustavohornig
Copy link

I'm having the same issue, any suggestions?

@i-like-robots
Copy link
Owner

I'm afraid I've not seen this myself, could you let me know which React version you are using?

@luciemac
Copy link
Author

luciemac commented Oct 5, 2018

I'm using React 16.3.2

@estherleah
Copy link

@i-like-robots I am also getting this problem.

@chellman
Copy link

I added a PR to change to defaultValue, but if you want a hack that avoids the warning until this is fixed, you can add a noop method to your class that uses ReactTags and reference it as the onChange for your component. It's ugly, but it will stop the warnings.

noOp() {
    // does nothing
}

<ReactTags
tags={this.state.tags}
suggestions={this.state.suggestions}
handleAddition={this.handleTagAddition}
handleDelete={this.handleTagDelete}
inputAttributes={{ onChange: this.noOp }}
/>

@i-like-robots
Copy link
Owner

i-like-robots commented Oct 18, 2018

This is an interesting issue, it was discussed at length some years back in facebook/react#1118 (although I've never seen this warning, with any version of React including 16.3.2!)

There is nothing technically wrong with the current implementation but I agree warnings cluttering up the console are annoying and should be avoided.

I'm not sure that setting defaultValue is the right solution as it may have unintended consequences (see #139 (comment)) but there are potentially several other workarounds available.

@i-like-robots
Copy link
Owner

i-like-robots commented Oct 18, 2018

Looking into this some more I can see that this warning has 5 conditions:

https://github.com/facebook/react/blob/b87aabdfe1b7461e7331abb3601d9e6bb27544bc/packages/react-dom/src/shared/ReactControlledValuePropTypes.js#L32-L46

It is the first condition which interests me because by default this should evaluate to true because the the default value for state.query is an empty string. This should therefore cause the function to return early and not output a warning.

Therefore to trigger this warning I had to change the default state (https://github.com/i-like-robots/react-tags/blob/master/lib/ReactTags.js#L35) to make it a truthy value.

Do any of you who are experiencing have an example which demonstrates the issue so I can look more closely at it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants