Create Synthetic Events Lazily #19909
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
gaearon
commented
Sep 25, 2020
| ): void { | ||
| const bubbleName = event._reactName; | ||
| const captureName = bubbleName !== null ? bubbleName + 'Capture' : null; | ||
| reactName: string, |
There was a problem hiding this comment.
This is not nullable now so we don't need extra checks below.
|
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 e3dde61:
|
gaearon
commented
Sep 25, 2020
| const listeners: Array<DispatchListener> = []; | ||
|
|
||
| const eventListeners = getEventHandlerListeners(currentTarget); | ||
| if (eventListeners !== null) { | ||
| const targetType = ((event.type: any): DOMEventName); |
There was a problem hiding this comment.
Not sure if the discrepancy (the code path above reads event.nativeEvent.type instead) was intentional but for now I'm keeping it.
gaearon
commented
Sep 25, 2020
Details of bundled changes.Comparing: 0a00804...e3dde61 react-dom
ReactDOM: size: 0.0%, gzip: -0.0% Size changes (experimental) |
Details of bundled changes.Comparing: 0a00804...e3dde61 react-dom
ReactDOM: size: 0.0%, gzip: -0.0% Size changes (stable) |
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently we create them very aggressively — especially with the eager change — but many may not end up being used. While the allocation itself is not a significant cost, we are actually running a bunch of code there, including iterating over the event interface, copying every known property into
this, and running our normalization polyfills. In some cases, our polyfills also do some other extra work (e.g. some calculations).However, this is just an artifact of the initial code structure. We don't actually need to create the synthetic events early except the enter/leave case where it's much more convenient. In other cases, we can make them lazy and create if we actually found some listeners. This also lets us delay any other extra work.
I should note that with a throttled CPU, the SyntheticEvent constructor takes a noticeable amount of time for noops, which is why I want to optimize this.