Skip to content

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.

);

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.

2 participants