Add test for warning for camelCased unknown props

[flarnie]

what is the change?:
When we render unknown props, they get converted to lower-case.

This may be unexpected for people, or break what they are expecting to
happen. Let's warn in this case and ask them to explicitly pass the
lower-cased attribute name.

why make this change?:
To avoid corner case buggy results for users.

test plan:
NOTE: ~~~It does NOT pass right now. This is a known issue.
Should we change and just expect a warning, but allow the attribute
value to be set?

yarn run test src/renderers/dom/shared/__tests__/ReactDOMAttribute-test.js

issue: #10399

[nhunzaker]

@flarnie do you mind if I push commits to fix this test?

[nhunzaker]

Oof. (edit: requiring lowercase) breaks a lot of tests that send in things like fooBar. Away I go!

[nhunzaker]

Should we change and just expect a warning, but allow the attribute value to be set?

Before I change quite a few tests, I'd vote we simply warn. What are other opinions? cc @sebmarkbage @gaearon (feel free to CC anyone else)

[flarnie]

@flarnie do you mind if I push commits to fix this test?

That would be great, and I agree we should decide on the desired behavior before jumping in.

Before I change quite a few tests, I'd vote we simply warn. What are other opinions? cc @sebmarkbage @gaearon (feel free to CC anyone else)

I think it would be ok to just warn, but also would like to get @sebmarkbage's opinion since he brought up this concern in the first place.

[sebmarkbage]

Even if we warn we wouldn't want the tests to trigger the warning so we should just fix the tests. (Unless the tests are actually testing something related to this.)

I think we should make this a hard console.error because we'd want to be able to change the behavior of these without requiring a major release. (For example, if one of them becomes known.)

[sebmarkbage]

This one seems to explicitly test this but that seems odd:

react/src/renderers/dom/shared/__tests__/ReactDOMServerIntegration-test.js

Lines 975 to 976 in 55209c8

	const e = await render(<div fooBar="test" />);
	expect(e.getAttribute('fooBar')).toBe('test');

This shouldn't pass...?

[sebmarkbage]

Oh I see, I guess the getAttribute itself is case-insensitive for HTML so it works. But element.attributes[0].name is lower-case.

[sebmarkbage]

Maybe this is isn't so important for HTML after all? Maybe it's just important for SVG? I forget why this came up last week. We could probably punt on this until after 16.0.

[sebmarkbage]

I think it might be enough to have this warning in SVG but that's very edge-casey so we could add it later if we don't want to block.

[nhunzaker]

I think it might be enough to have this warning in SVG but that's very edge-casey so we could add it later if we don't want to block.

I'm on board with this. At this point, ReactDOMUnknownPropertyHook could use a refactor. The interaction between it, ReactDOMFiberComponent, and others pieces feels ripe for it. It would be great to revisit how we track what is an HTML, SVG, custom attribute, etc, and rethink these validations with less of a constraint on time and without the burden of the Stack renderer.

[gaearon]

Yes, I think it was only important for SVG.

[gaearon]

Even if we add this warning I don't think we plan to strip them. Rather, we'll warn but leave them in the DOM.

[gaearon]

I sent a PR for this in #10699.