Skip to content

Conversation

@aweary
Copy link
Contributor

@aweary aweary commented Nov 20, 2016

See discussion starting here.

This prevents the fake event used here from being passed to the callback. This was causing issues where event handlers received an extra argument in dev only.

@gaearon I verified this fix resolves the issue by building and testing in the basic click counter example. I'm not sure where a unit test for this would go.

This way the fake event isn't being implicitly passed into the event handler
@gaearon
Copy link
Collaborator

gaearon commented Nov 20, 2016

Feel free to add a test for ReactErrorUtils if one doesn't exist already. You could also look at change history to see if there were any related tests introduced with this file in the first place.

Can you confirm single argument matches production behavior?

@aweary
Copy link
Contributor Author

aweary commented Nov 28, 2016

@gaearon I added test for ReactErrorUtils 👍

@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2016

Lint is failing.

@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2016

Tagging as minor since somebody might have accidentally relied on that.

@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2016

Linter fails.

Add fiber test report

Linting fixes
@aweary aweary force-pushed the dev-event-handler-args branch from 996e59d to f293884 Compare November 28, 2016 16:31
@aweary
Copy link
Contributor Author

aweary commented Nov 28, 2016

@gaearon fixed 👍

@gaearon gaearon merged commit 6947db1 into facebook:master Nov 28, 2016
@gaearon
Copy link
Collaborator

gaearon commented Nov 28, 2016

LGTM

@gaearon gaearon modified the milestones: 15-hipri, 15-lopri Jan 6, 2017
@aweary aweary deleted the dev-event-handler-args branch January 26, 2017 16:18
acusti pushed a commit to brandcast/react that referenced this pull request Mar 15, 2017
…ok#8363)

* Use a closure to bind gaurded callback

This way the fake event isn't being implicitly passed into the event handler

* Add tests for ReactErrorUtils

Add fiber test report

Linting fixes
@aweary aweary mentioned this pull request Jul 11, 2017
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants