ReactDOMEventListener: clean up module #18713
Conversation
facebook-github-bot
added
CLA Signed
React Core Team
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 481e2d7:
|
trueadm
force-pushed
the
clear-up-event-listener
branch
from
55f46ed to
481e2d7
Compare
| @@ -53,7 +54,11 @@ describe('ChangeEventPlugin', () => { | |||
| postMessageCallback(postMessageEvent); | |||
| } | |||
| }; | |||
| jest.resetModules(); | |||
| ReactFeatureFlags.enableModernEventSystem = true; | |||
There was a problem hiding this comment.
I think... you don't need this test to be .internal anymore since the enableModernEventSystem flag is marked as variant in ReactFeatureFlags.www-dynamic.
I think you should be able to remove the .internal suffix on this (and the below test) and just use the new pragma comment:
// @gate enableModernEventSystemThere was a problem hiding this comment.
I tried this locally just now, for some reason some tests in files still break with this change. Namely EnterLeave. To make sure I'm sane, I break the old legacy event system whilst running the change, so I can see if we are using the modern system or the legacy – and it seems we are still using the legacy with // @gate enableModernEventSystem
I'd rather tackle why that might be the case in another PR. :)
This PR tidies up
ReactDOMEventListenerand moves the different event listener approaches into their respective modules. This makes it easier to remove event systems in the future without breaking something else. Along the way, I noticed that we weren't applying the modern event system flag in the plugin tests properly, so this fixes that too.