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.

Sign up for GitHub

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

Jump to bottom

Make sure simulated events don't warn when providing extra event properties #6380

Merged
merged 1 commit into from
Apr 6, 2016
Merged

Make sure simulated events don't warn when providing extra event properties #6380

merged 1 commit into from
Apr 6, 2016
+30 −1

Conversation

zpao
Copy link
Member

@zpao zpao commented Mar 30, 2016

fixes #6379

@zpao
Copy link
Member Author

zpao commented Mar 30, 2016

I think in an ideal world we'd be using the right SyntheticEvent subclass constructor for the event. This would allow us to set the properties directly on the fakeNativeEvent before passing it to the constructor, then all the appropriate properties would be copied off fakeNativeEvent (Eg, a click event would go through SyntheticMouseEvent and look at the MouseEventInterface which includes clientX). This would require more work though so I didn't want to get into it now.

gaearon
gaearon reviewed Mar 30, 2016
View reviewed changes
src/test/__tests__/ReactTestUtils-test.js
@@ -474,6 +474,29 @@ describe('ReactTestUtils', function() {
expect(handler).not.toHaveBeenCalled();
});

it.only('should not warn when simulating events with extra properties', function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

.only

@gaearon
Copy link
Collaborator

gaearon commented Mar 30, 2016

Re: ideal world, relevant to #6154.

@zpao zpao force-pushed the simulatewarning branch from 7c1e70f to c9bd9e2 Compare March 30, 2016 20:32
@facebook-github-bot
Copy link

@zpao updated the pull request.

@zpao zpao added this to the 15.0 milestone Mar 31, 2016
@facebook-github-bot
Copy link

@zpao updated the pull request.

@zpao zpao force-pushed the simulatewarning branch from c9bd9e2 to 69c91b7 Compare March 31, 2016 20:44
@zpao
Copy link
Member Author

zpao commented Mar 31, 2016

Hmm, worth noting that this silences an expected warning (easier to see now on travis now that other issues are cleaned up). I'll look into that some more…

@facebook-github-bot
Copy link

@zpao updated the pull request.

@zpao zpao mentioned this pull request Apr 4, 2016
Closed
1 task
@zpao zpao force-pushed the simulatewarning branch from 69c91b7 to 86d9a9b Compare April 5, 2016 21:47
@zpao zpao force-pushed the simulatewarning branch from 86d9a9b to 42b1cba Compare April 5, 2016 22:06
@zpao zpao mentioned this pull request Apr 5, 2016
Closed
15 tasks
@sebmarkbage
Copy link
Collaborator

Is the travis test error relevant? Looks plausible.

@zpao
Copy link
Member Author

zpao commented Apr 6, 2016

Nope, that's because of #6228. Will take a closer look at that test (only failing in non-createElement mode) in the morning but this should be fine.

@jimfb
Copy link
Contributor

jimfb commented Apr 6, 2016

👍

@zpao zpao merged commit 65c68b5 into facebook:master Apr 6, 2016
@aweary aweary mentioned this pull request Jun 23, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed
Projects
None yet
Milestone
15.0
Development

Successfully merging this pull request may close these issues.

ReactTestUtils.Simulate logs unexpected console error
5 participants
@zpao @gaearon @facebook-github-bot @sebmarkbage @jimfb