React: `HTMLDOMPropertyConfig` contains non-standard `icon` property

[smockle]

Do you want to request a _feature_ or report a _bug_?
Report a bug.

What is the current behavior?
The HTMLDOMPropertyConfig object includes an icon property in a section title Standard Properties, but icon isn’t an attribute supported by any standard HTML element.

icon is not included in the HTML4 or HTML5 specs:
List of HTML4 attributes
List of HTML5 attributes

Mozilla Developer Network includes icon in its list of attributes, but specifies the only element to use icon is the obsolete <command> element. <command> isn’t supported in any desktop or mobile browser.

What is the expected behavior?
The icon property might be better placed in the section titled Non-standard Properties, or removed altogether. The documentation for HTML attributes could omit icon too.

I encountered this issue when working with TypeScript, and I opened an issue about the React type definition at https://github.com/DefinitelyTyped/DefinitelyTyped/issues/10467. Because React itself includes icon, custom interfaces which extend DefinitelyTyped’s React.HTMLAttributes cannot include a non-string icon property.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
icon was added in a React 0.4.0 alpha in 2013: https://github.com/facebook/react/commit/203dba271b6128f03c99b1ce2f573a316adb4e4e. The commit message mentions mostly this was to better support some HTML5 stuff—it’s possible icon was part of a draft of the HTML5 standard at the time, but was omitted in the final spec.

[zpao]

Thanks for spotting this. We should probably remove it but it'll have be done in a major version.

[nhunzaker]

This would become moot if already removing the whitelist, right?

[zpao]

Correct, just not clear exactly when that happens so it would be fine to take a smaller change like this first. Removing the whitelist would also likely be a major version change.

[smockle]

Thanks for taking a look at this! I’d be happy to open a PR.

From the contribute section of the README, it looks like I’d just need to remove the line, ensure tests pass & linter output is clean, then submit the PR? (I want to make sure I’m not missing a version or changelog step.)

Edit: From reading the CONTRIBUTING doc (specifically step 3), I‘d assume an update to the Tags and Attributes page (and localizations) should be included in the PR too.

[aweary]

@smockle that's about all it takes. People don't usually update all the localizations, but since this should be easy to update in all languages it would be great if you could do that 👍

[zpao]

You can actually leave the docs part of the change out. We usually just generate those manually around releases (makes it a bit easier to batch so line lengths are reasonable)

[aweary]

Oh, good to know, didn't realize they were generated 👍

[smockle]

Thanks! I just submitted PR https://github.com/facebook/react/pull/7508 to address this issue. Originally the PR also updated the docs, but per this comment I’ve removed that commit. All tests pass, and there are no linter errors.