Use lower case for HTML attributes

[gaearon]

It doesn't really matter because HTML is case insensitive but people prefer it.
Fixes #10863.

[gaearon]

Also verified the attribute table hasn’t changed.

[nhunzaker]

Should this happen once, when the properties are injected?

[gaearon]

That wouldn't help with properties that aren't in the whitelist.

[nhunzaker]

But line 93 ensures that this code only runs when we have property info (which I believe is always from the whitelist)

[gaearon]

Ah I see. I guess we should update the custom ones too in this case.

[nhunzaker]

Wait. Why is this necessary? This already gets lower cased in the injection here:
https://github.com/facebook/react/blob/master/src/renderers/dom/shared/DOMProperty.js#L83

[nhunzaker]

Looks like tests pass when I remove this statement! (98-100). Totally need the toLowerCase below though.

[nhunzaker]

Looks good, but if we can run toLowerCase when we inject property info ahead of time, that could be nice. That might require passing the namespace into the injection function, or specifying it in the injections themselves.

[reactjs-bot]

Deploy preview ready!

Built with commit 850550c

https://deploy-preview-11110--reactjs.netlify.com

[gaearon]

I'm still not super sure it's worth it. I wonder if we can measure perf impact somehow.

[nhunzaker]

@gaearon I wonder what it would take to more easily measure that sort of thing. We have the benchmarks, but do they cover SSR? @aickin did some work here. I wonder if we could adopt some of it.

Still it intuitively doesn't seem substantial.

[syranide]

Isn't it better to warn instead? It avoids confusion and possible spreading headaches.

<div fooBar={1} {...{foobar: 2}} />

But I'm guessing there's more to it than that.

[gaearon]

It's mostly about lower casing supported attributes.

[sebmarkbage]

If this is meant for supported attributes, I think we should only do it for them. Since those are a few special cases we can compare them using interned strings (which we probably do in some places anyway?) and omit the same lower-cased interned string for each match instead of calling toLowerCase.

For custom ones we could warn but might be excessive since there might be custom ones where this is legit (although probably not).

[syranide]

@sebmarkbage https://html.spec.whatwg.org/multipage/custom-elements.html#custom-elements-core-concepts

Any namespace-less attribute that is relevant to the element's functioning, as determined by the element's author, may be specified on an autonomous custom element, so long as the attribute name is XML-compatible and contains no ASCII upper alphas. The exception is the is attribute, which must not be specified on an autonomous custom element (and which will have no effect if it is).

So with the exception of namespaced attributes, custom elements may not use upper-case characters either.

[cosmicdreams]

Do the conflicting files mean this work needs to be rebased? Sorry, new to this.

[gaearon]

Yes. But we also haven't decided if we actually want to do this.

[gaearon]

Meh. What we're doing is technically correct. If somebody feels strongly about the output they can prettify it themselves. In practice it doesn't matter, and we don't want to take any potential perf hit there. If someone feels strongly you're welcome to provide real-world measurements demonstrating this patch has no effect on perf.