Skip to content
This repository was archived by the owner on Jul 30, 2020. It is now read-only.

Commit 6016766

Browse files
committed
feat: ignore events on diabled targets
Previously, an event could still fire if its target was disabled. This is incorrect because that wouldn't work in a real application. Additionally, undocumented APIs for querying React elements were removed in the sprit of the testing-library guiding principles.
1 parent 0c634a3 commit 6016766

File tree

4 files changed

+61
-9
lines changed

4 files changed

+61
-9
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`examples of some things 1`] = `undefined`;

src/__tests__/events.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ test('calling a handler when there is no valid target throws', () => {
5555
expect(handleEvent).toBeCalledTimes(0);
5656
});
5757

58+
test('calling a handler the target is disabled throws', () => {
59+
const handleEvent = jest.fn();
60+
const { getByText } = render(<Button disabled onPress={handleEvent} title="button" />);
61+
expect(() => fireEvent.press(getByText('button'))).toThrow();
62+
expect(handleEvent).toBeCalledTimes(0);
63+
});
64+
5865
test('calling an event that has no defined handler throws', () => {
5966
const { getByText } = render(<Text>test</Text>);
6067
expect(() => fireEvent.press(getByText('test'))).toThrow();

src/events.js

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,15 @@ const eventMap = {
202202
},
203203
};
204204

205+
const disableableElements = [
206+
'Slider',
207+
'Switch',
208+
'TouchableHighlight',
209+
'TouchableNativeFeedback',
210+
'TouchableOpacity',
211+
'TouchableWithoutFeedback',
212+
];
213+
205214
class NativeEvent {
206215
constructor(typeArg, event = {}) {
207216
const config = eventMap[typeArg] || eventMap.custom;
@@ -225,25 +234,37 @@ function getEventHandlerName(key) {
225234
return `on${key.charAt(0).toUpperCase()}${key.slice(1)}`;
226235
}
227236

237+
function validateElementType(list, element) {
238+
return (
239+
list.includes(element.type) ||
240+
list.includes(element.type.name) ||
241+
list.includes(element.type.displayName)
242+
);
243+
}
244+
228245
function isValidTarget(element, event) {
229-
return event.validTargets.length
230-
? event.validTargets.includes(element.type) ||
231-
event.validTargets.includes(element.type.name) ||
232-
event.validTargets.includes(element.type.displayName)
233-
: true;
246+
return event.validTargets.length ? validateElementType(event.validTargets, element) : true;
247+
}
248+
249+
function isDisabled(element) {
250+
return element.props.disabled && validateElementType(disableableElements, element);
234251
}
235252

236253
function findEventHandler(element, event) {
237254
const { typeArg } = event;
238255
const eventHandler = getEventHandlerName(typeArg);
239-
const isValid = isValidTarget(element, event);
256+
const valid = isValidTarget(element, event);
257+
const disabled = isDisabled(element);
240258

241-
if (typeof element.props[eventHandler] === 'function' && isValid) {
259+
if (typeof element.props[eventHandler] === 'function' && valid) {
260+
if (disabled) {
261+
throw new Error(`A target was found for event: "${typeArg}", but the target is disabled!`);
262+
}
242263
return element.props[eventHandler];
243264
}
244265

245266
if (element.parent === null || element.parent.parent === null) {
246-
throw new Error(`No handler found for event: ${typeArg}`);
267+
throw new Error(`No target found for event: "${typeArg}"`);
247268
}
248269

249270
return findEventHandler(element.parent, event);

src/queries.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,28 @@ function queryAllByText(
3131
[],
3232
);
3333

34-
return [...baseArray].filter(node => matcher(getNodeText(node), node, text, matchNormalizer));
34+
return [...baseArray]
35+
.filter(node => matcher(getNodeText(node), node, text, matchNormalizer))
36+
.map(node => {
37+
// We take the guiding principles seriously around these parts. These methods just let
38+
// you do too much unfortunately, and they make it hard to follow the rules of the
39+
// testing-library. It's not that we don't trust you, in fact we do trust you! We've
40+
// left `findAll` on the instance for you as an emergency escape hatch for when
41+
// you're really in a bind. We do want you to test successfully, after all ☺️
42+
//
43+
// The main intent is to:
44+
// 1) Make it hard to assert on implementation details
45+
// 2) Force you to consider how to test from a user's perspective
46+
//
47+
// Of course if you can't figure that out, we still want you to be able to use this library,
48+
// so use `findAll`, just use it sparingly! We really believe you can do everything you
49+
// need using the query methods provided on the `render` API.
50+
['find', 'findAllByProps', 'findAllByType', 'findByProps', 'findByType', 'instance'].forEach(
51+
op => Object.defineProperty(node, op, { get: undefined }),
52+
);
53+
54+
return node;
55+
});
3556
}
3657

3758
function queryByText(...args) {

0 commit comments

Comments
 (0)