Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use accumulateTwoPhaseDispatchesSingle directly #18203

Merged
merged 1 commit into from
Mar 5, 2020

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Mar 3, 2020

I noticed this when working on the modern event system, so thought I'd add the improvement separately.

We only accumulate two phase dispatches on a single event. So passing it to accumulateTwoPhaseDispatches is unecessary in a bunch of places and is just overhead and runtime indirection.

Furthermore, with the modern event system we may want to add an additonal argument to accumulateTwoPhaseDispatchesSingle during event plugin phase, so this makes that far simpler to do.

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Mar 3, 2020
@codesandbox-ci
Copy link

codesandbox-ci bot commented Mar 3, 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 9921ddd:

Sandbox Source
polished-pine-nld42 Configuration

@sizebot
Copy link

sizebot commented Mar 3, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 9921ddd

@sizebot
Copy link

sizebot commented Mar 3, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 9921ddd

Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stamp. the code looks fine, but i'm unaware of the specific consequences. I'll defer to you on that.

@trueadm trueadm merged commit 2fe0fbb into facebook:master Mar 5, 2020
@trueadm trueadm deleted the improve-propagators branch March 5, 2020 00:04
@gaearon
Copy link
Collaborator

gaearon commented Mar 5, 2020

i'm unaware of the specific consequences

If you're curious — each accumulate* function has a accumulate*Single version. The difference as I understand is that accumulate*Single accepts an event, but accumulate* accepts either event or an array. Since we're calling it with event alone in those cases, there was no need to call the other version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants