Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upRemove TestUtils dependency on event registry #19235
Merged
+109
−26
Conversation
| @@ -40,15 +35,7 @@ function publishEventForPlugin( | |||
| eventTypes: EventTypes, | |||
| eventName: string, | |||
| ): boolean { | |||
| invariant( | |||
This comment has been minimized.
This comment has been minimized.
gaearon
Jul 1, 2020
Author
Member
While this invariant is useful to us in theory we have a similar one that would catch the same issue below anyway. We should also probably make that one a DEV-only warning.
| const dispatchConfig = eventNameDispatchConfigs[eventType]; | ||
| // Reconstruct more or less what the original event system produced. | ||
| // We could remove this indirection here but we also don't plan to invest in Simulate anyway. | ||
| const dispatchConfig = {}; |
This comment has been minimized.
This comment has been minimized.
gaearon
Jul 1, 2020
Author
Member
While a real dispatch config would have more fields, they're not used by TestUtils.
| @@ -549,6 +548,13 @@ function accumulateTwoPhaseDispatchesSingle(event) { | |||
|
|
|||
| const Simulate = {}; | |||
|
|
|||
| const directDispatchEventTypes = new Set([ | |||
This comment has been minimized.
This comment has been minimized.
gaearon
Jul 1, 2020
Author
Member
Not sure this actually makes sense since those should have a special enter-leave traversal? But this is what the code is already doing.
codesandbox
bot
commented
Jul 1, 2020
|
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 8dfb080:
|
sizebot
commented
Jul 1, 2020
Details of bundled changes.Comparing: 9e7f5c0...8dfb080 react-dom
ReactDOM: size: Size changes (stable) |
sizebot
commented
Jul 1, 2020
Details of bundled changes.Comparing: 9e7f5c0...8dfb080 react-dom
ReactDOM: size: Size changes (experimental) |
|
Ignore the size alert, it's talking about TestUtils. |
gaearon
merged commit b683c07
into
facebook:master
32 checks passed
32 checks passed
ci/circleci: RELEASE_CHANNEL_stable_yarn_test_prod_www_variant
Your tests passed on CircleCI!
Details
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
gaearon commentedJul 1, 2020
This removes the last TestUtils link to the event system internals.
The dispatch config map was used to populate the Simulate helpers. I just took a snapshot of the list we have today, and inlined it in TestUtils. We also used the config to tell whether an event is two-phased or not. However there are only four events that don't have a capture phase so I have explicitly listed them instead.
In the future we should be able to remove more of these intermediate objects in the event system itself.