Attach event per react container root, rather than on the document
chenglou opened this issue · 9 comments
@nathansobo will that help your event perf issues a bit? I'm not familiar with atom's plugin infrastructure, But this'll help if you have <Editor/><Plugin1/> (two renderComponents). Doesn't help if you have <Editor><Plugin1/></Editor> though, but I have some ideas to optimize events a bit more.
This is nonetheless an ok idea, I think.
@spicyj
@chenglou That would help our situation with the editor a bit, yes. But what's more important for Atom I think is the ability to integrate React with HTML custom elements without requiring the use of the shadow DOM. Seems like #1711 would help that, and it seems like this idea might be necessary to achieve that as well?
@nathansobo Can you clarify what you mean, "without requiring the use of the shadow DOM"? I'm a bit uninformed about how HTML custom elements work right now.
@syranide I don't see why we should close issues just because there's a PR open – what if we don't go with that particular implementation? We may still want to make the same fix.
As far as I can tell #8117 was the latest attempt to do this. It has requested changes (that weren't addressed). So if somebody wants to pick this up they'll probably want to read the discussion there.
Just FYI, this effort may get new life due to: testing-library/react-testing-library#35
If I can't push this along myself, I'll try to encourage others to do so. Thanks!
@trueadm has been working on this. Not yet 100% sure but it's likely we'll do this change in a major (right now it's behind a flag).