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

Generate event object for fireEvent #1171

Closed

Conversation

CodingItWrong
Copy link
Contributor

@CodingItWrong CodingItWrong commented Oct 14, 2022

HT to @jlcampbell and @MLee21 for collaborating with me on this investigation and PR!

Summary

This PR shows initial work toward implementing #714. It is not yet ready to merge: additional questions are posted in the issue.

Test plan

Run fireEvent.ts and note that the tests pass.

@@ -56,6 +56,28 @@ const CustomEventComponentWithCustomName = ({
);

describe('fireEvent', () => {
test('QUESTION: is there a kind of event that takes multiple arguments?', () => {
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK events by convention take a single argument of Event type or subtype. We also have onChangeText which gets a single string value, but that seems to be simplified version of onChange. I would assume that we can focus only on handling single argument. That should simplify the typing, while we can always refactor if there would be any such event.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this info! BTW I noticed that I didn't clearly link to our additional questions from here. Here they are: #714 (comment)

Copy link
Member

@mdjastrzebski mdjastrzebski left a comment

Choose a reason for hiding this comment

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

So far, so good! I think it would be wise to start by researching real RN app behaviour and coding that into unit tests.

src/fireEvent.ts Outdated
Comment on lines 119 to 129
const generatedEventObject = { someKey: 'value' };

let defaultCallbackValues =
eventName === 'changeText' ? [] : [generatedEventObject];
const handlerCallbackValues = data.length > 0 ? data : defaultCallbackValues;

let returnValue;

act(() => {
returnValue = handler(...data);
returnValue = handler(...handlerCallbackValues);
});
Copy link
Member

Choose a reason for hiding this comment

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

Generally we would like to follow DTL's fireEvent implementation, with adjustments necessary to account for differences between RN and React DOM.

  1. They have event map record which holds mapping from event name to event object constructor name and some default init props: https://github.com/testing-library/dom-testing-library/blob/main/src/event-map.js
  2. They only apply default event iff someone uses fireEvent.xxx. They do not seem to apply in when using fireEvent('xxx'):
  1. Important parts seems to be dispatching that event in context of given node. See: https://github.com/testing-library/dom-testing-library/blob/edffb7c5ec2e4afd7f6bedf842c669ddbfb73297/src/events.js#L17

In DTL they have element.displatchEvent, but in ourcase it seems that we have to manually assign target property of event, since our ReactTestInstance does not implement EventTarget which achieves that effect automatically for RTL.

@mdjastrzebski
Copy link
Member

Superseded by #1258

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants