Remove event namespace check from ReactPartialRenderer

[nhunzaker]

Working through #10229, I noticed that onchange was coming through as markup in the server-side rendering tests. This is because ReactPartialRenderer uses a registrationNameModules check to see if a prop is an event name.

I believe registrationNameModules is populated by event plugins, which were removed in #10173. So this condition always resolves to true. I've confirmed by running test coverage:

If we remove the attribute whitelist, we need an answer here for event names on server-side rendering. I've added that to my list in #10229

[nhunzaker]

I wonder what happens if you pass onChange to a web component. Hmmm. I'll investigate.

[gaearon]

Related to #10272.

[gaearon]

Does this fix the issue? Or would this PR only work if we passed attributes through or something like this? Can you explain more about what this PR does to address this?

[nhunzaker]

My original description wasn't clear. I apologize for that.

EventPluginRegistry.registrationNameModules is an empty object because no event plugins are injected for the server build. This means that the check to rule out event handler properties always returns true. This allows event handler properties (like onChange) to pass through to DOMMarkupOperations to be written to the element.

This doesn't happen because no property info exists for event handlers, so the attribute whitelist check in DOMMarkup Operations prevents an error. No property info is found, and the elements in the tests are not custom elements (as far as I know) so no attribute is written.

I am curious what would happen if you wrote an event listener on a custom attribute when server rendering. Like <my-widget onChange={doStuff} /> Would that write out to the string "<my-widget onchange={function doStuff() {}}></my-widget>?

I am happy to convert this into an issue and close out this PR if someone else would like to tackle this.

[nhunzaker]

In any case, it sounds like this is definitely not desired behavior. This PR was intended to remove, what I thought, was an unnecessary check and only later did realize that this might be an actual issue.

What if the top level event types list was built in a more general purpose config instead of in SimpleEventPlugin and co?

[gaearon]

I think I get it now. This doesn’t expose a bug now but if we stripped the whitelist, we’d have to tackle this problem.

I’ll get this in because it simplifies the current behavior. We just have to be mindful of how we want to proceed in the future. I’m open to suggestions.

[nhunzaker]

@gaearon Cool. I've added it as a task in #10229. FWIW, the test suite produces about 40 failed tests if event attribute names get through to the element.