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

Group warnings for unknown DOM properties #7152

Closed
gaearon opened this issue Jun 30, 2016 · 11 comments
Closed

Group warnings for unknown DOM properties #7152

gaearon opened this issue Jun 30, 2016 · 11 comments

Comments

@gaearon
Copy link
Collaborator

gaearon commented Jun 30, 2016

I just ran React Bootstrap doc page on master and saw a ton of warnings from #6800:

jun 30 2016 01 55

That page is really long so maybe it’s an edge case, but at the very least I think we should group props from the same element into a single warning. For example, these warnings could become one:

screen shot 2016-06-30 at 01 53 11

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 30, 2016

If you’d like to work on this please comment in this thread so others know you’re taking it.
This is relatively high priority so please also comment if you abandon this, so others can pick it up.
Thanks!

@griffinmichl
Copy link
Contributor

I'd love to take a crack at this.

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 30, 2016

@gm758 You got it! Let me know if you have any questions. #6800 would be a good reference for the warning we added—we just need to change it to accumulate “bad” props before emitting a single warning outside the loop.

@gaearon
Copy link
Collaborator Author

gaearon commented Jun 30, 2016

Fixed via #7153

@gaearon gaearon closed this as completed Jun 30, 2016
@kennyhk
Copy link

kennyhk commented Jul 4, 2016

hello, please can anyone tell me how to disable this warning? thank you!

@suhaotian
Copy link

suhaotian commented Jul 4, 2016

@kennyhk I also want to disable this warning 😢

@gaearon
Copy link
Collaborator Author

gaearon commented Jul 4, 2016

You can't disable it. We don't add warnings just to make your life harder. It warns about a problematic pattern that will behave unexpectedly in the next major version. You need to fix this. If you use third party libraries and warnings come from them, you need to file issues with those libraries so they get fixed.

Please read about warning and the discussion below: https://gist.github.com/jimfb/d99e0678e9da715ccf6454961ef04d1b

@akre54
Copy link
Contributor

akre54 commented Jul 12, 2016

We load SVG files as react components using the svg-react-loader.

The SVGs come from Illustrator and have an xmlns prop, which React doesn't recognize. In your opinion is it the library's responsibility to keep up to date with which props React supports? Is it our responsibility to remove any props React doesn't like? Should React support xmlns? What's the solution here?

@gaearon
Copy link
Collaborator Author

gaearon commented Jul 12, 2016

I think support for xmlns has been merged just a few days ago. So it'll be out in the next version.

@akre54
Copy link
Contributor

akre54 commented Jul 12, 2016

Awesome to hear. Thanks.

For the future, is the right solution to try to keep React's list of accepted props complete and up-to-date?

@zpao
Copy link
Member

zpao commented Jul 12, 2016

Yes, however we should have pretty much everything from SVG and HTML. Hopefully there isn't much we missed but I fully expect new specs to come out and for us to need to handle that.

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

6 participants