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

Select warning fires repeatedly #11795

Closed
arackaf opened this issue Dec 7, 2017 · 11 comments
Closed

Select warning fires repeatedly #11795

arackaf opened this issue Dec 7, 2017 · 11 comments

Comments

@arackaf
Copy link

arackaf commented Dec 7, 2017

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

See fiddle https://jsfiddle.net/j2nzg31L/1/

The warning related to the select fires repeatedly. @gaearon tells me it should dedupe, and that this is a bug.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template for React 16: https://jsfiddle.net/Luktwrdm/, template for React 15: https://jsfiddle.net/hmbg7e9w/).

See above

What is the expected behavior?

See above

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.2

@gaearon
Copy link
Collaborator

gaearon commented Dec 7, 2017

We should deduplicate this warning (like we do with many other warnings).

@watadarkstar
Copy link
Contributor

watadarkstar commented Dec 7, 2017

@gaearon Hi Dan, I can take this. I imagine its similar to: #11113 and #11216

@gaearon
Copy link
Collaborator

gaearon commented Dec 7, 2017

Yep. Sounds good!

@watadarkstar
Copy link
Contributor

@gaearon

Hi Dan, I wanted to check with you that I am on the right track before I try writing any code.

So here is the warning:

export function validateProps(element: Element, props: Object) {
// TODO (yungsters): Remove support for `selected` in <option>.
if (__DEV__) {
warning(
props.selected == null,
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.',
);
}
}

I'm not sure if I should be writing a test in:

And I don't see a test that already tests for this warning - am I missing something? Or should I write a entirely new test in one of those files?

I haven't worked with the test suite before so I'm not sure how its supposed to be organized.

@gaearon
Copy link
Collaborator

gaearon commented Dec 9, 2017

I don't think it matters which file you write it in.

To verify if there are any existing tests, you can just comment out the warning and run npm test. If nothing fails, well, yeah, then we didn't have tests for it.

@gaearon
Copy link
Collaborator

gaearon commented Dec 10, 2017

Thanks!

@arackaf
Copy link
Author

arackaf commented Dec 10, 2017

Wow that was incredibly fast - great work ❤️

@watadarkstar
Copy link
Contributor

watadarkstar commented Dec 10, 2017

@arackaf No problem, but I couldn't have done it without Dan's super fast support (I swear he is not human answering all our tweets and github comments instantly) 😜

@blling
Copy link

blling commented Dec 10, 2017

OMG, Dan has no weekend!

@gaearon
Copy link
Collaborator

gaearon commented Dec 10, 2017

It’s really cold outside and I’m too bored :-) I’ll go do something else now.

@watadarkstar
Copy link
Contributor

Lol no Dan don't leave us! 😰

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

4 participants