Add `testing-library` compatibility props by latekvo · Pull Request #3357 · software-mansion/react-native-gesture-handler

latekvo · 2025-01-27T15:27:19Z

Description

This PR adds 3 props to the Pressable used for compatibility with react-native-testing-library

Full discussion: callstack/react-native-testing-library#1738

Test plan

mdjastrzebski · 2025-01-27T16:03:31Z

src/components/GestureButtonsProps.ts

+   * Used for testing-library compatibility, not passed to the native component.
+   */
+  // eslint-disable-next-line @typescript-eslint/ban-types
+  testOnly_onPress?: Function | null;


What kind of argument should be passed to testOnly_onPress?

testOnly_onPress will currently always be of type ((event: PressableEvent) => void) | null, but this may change if more of our components will implement testOnly_onPress.

I've made it of type Function, because RawButtonProps usage is not specific to Pressable.

Link to type definitions: (link)
Extract from type definitions:

export type InnerPressableEvent = { changedTouches: InnerPressableEvent[]; identifier: number; locationX: number; locationY: number; pageX: number; pageY: number; target: number; timestamp: number; touches: InnerPressableEvent[]; force?: number; }; export type PressableEvent = { nativeEvent: InnerPressableEvent }; export interface PressableProps extends AccessibilityProps, Omit<ViewProps, 'children' | 'style' | 'hitSlop'> { [...] onPress?: null | ((event: PressableEvent) => void); onPressIn?: null | ((event: PressableEvent) => void); onPressOut?: null | ((event: PressableEvent) => void); [...] }

mdjastrzebski · 2025-01-27T16:06:00Z

src/components/Pressable/Pressable.tsx

      ? children({ pressed: pressedState })
      : children;

+  const inJestEnv = typeof jest !== 'undefined';


Perhaps we should use process.env.NODE_ENV === 'test'; as more typical option for checking test env.

Fixed in e51fdcd

We already have isJest function (defined here). Can't we use that?

NODE_ENV === 'test' also works with Mocha and Jasmine, while isJest which uses JEST_WORKER_ID only works with Jest. I think it'd actually be worth changing isJest function to the implementation present in this PR.

In the future RNTL would like to support other test runners, like vitest or bun. If you hardcode to Jest instead of testing, this would break in such case.

In that case why don't we change isJestEnv implementation instead of adding new check?

I agree the isJestEnv implementation can be changed, mentioned this in #3357 (comment) as well.
Done in f13e7b0

mdjastrzebski

Generally looks good, I've left some recommended tweaks

….com/software-mansion/react-native-gesture-handler into @latekvo/add-testing-library-support

latekvo · 2025-01-27T17:39:32Z

Note on c63a561 - I tried using ReturnType<typeof setTimeout>, manually creating globals.d.ts type resolutions, and finding ways of giving one of the typing libraries higher priority. Unfortunately none of these methods worked for all instances of conflicting types, with node types always overwriting react-native types.

j-piasecki · 2025-01-28T10:36:22Z

src/components/Pressable/Pressable.tsx


+  // @ts-ignore Adding type definitions for @types/node causes multiple conflicts with react-native/types,
+  //            and as far as i know, there is no simple way of automatically resolving them.
+  const inTestEnv = process.env.NODE_ENV === 'test';


Can we use this

react-native-gesture-handler/src/utils.ts

Lines 37 to 40 in 5b535c9

export function isJestEnv(): boolean {

// @ts-ignore Do not use `@types/node` because it will prioritise Node types over RN types which breaks the types (ex. setTimeout) in React Native projects.

return hasProperty(global, 'process') && !!process.env.JEST_WORKER_ID;

}

?

Mentioned here: #3357 (comment)

THis would tie the implementation only to Jest test runner. In case of user using other test runners Mocha, Vitest, Bun (not all of these are supported by RNTL atm) this props will not be set. The process.env.NODE_ENV === 'test'; should support all test runners.

Done in 4d942e8

mdjastrzebski · 2025-01-28T15:46:07Z

@latekvo could you also expose onLongPress in a similar way?

mdjastrzebski · 2025-01-28T15:47:14Z

I've implemented RNTL counterpart here: callstack/react-native-testing-library#1741

m-bert · 2025-01-28T15:50:56Z

src/components/Pressable/Pressable.tsx

+        testOnly_onPress={isTestEnv() ? onPress : undefined}
+        testOnly_onPressIn={isTestEnv() ? onPressIn : undefined}
+        testOnly_onPressOut={isTestEnv() ? onPressOut : undefined}>


We could do something similar to Reanimated, i.e. call this only once and store result in constant

const IS_TEST_ENV = isTestEnv()

I though about that, but isTestEnv() is just a property read, and both notations are just as readable, so i don't think there's a point in caching the value.

Well, it calls hasProperty and then compares values, so it's more than just property read. While it's not crucial in test environment, I think it is better to memoize this value.

You're right, done in 7a31f5e and 4ec26c4

latekvo · 2025-01-28T16:31:53Z

@latekvo could you also expose onLongPress in a similar way?

@mdjastrzebski sure, done in 6184078

mdjastrzebski

LGTM 🙌

mdjastrzebski · 2025-02-03T13:50:46Z

@latekvo could you make a release with this PR?

latekvo · 2025-02-05T15:12:54Z

@mdjastrzebski, FYI, this PR has just been released in 2.23.0.

add testOnly onPressX props to Pressable

122d977

latekvo mentioned this pull request Jan 27, 2025

Improve react-native-gesture-handler::pressable handling callstack/react-native-testing-library#1738

Closed

mdjastrzebski reviewed Jan 27, 2025

View reviewed changes

latekvo added 5 commits January 27, 2025 17:37

update test env detection

e51fdcd

Merge branch 'main' into @latekvo/add-testing-library-support

9828131

add node type defs

930dabc

Merge branch '@latekvo/add-testing-library-support' of https://github…

f1ac3ce

….com/software-mansion/react-native-gesture-handler into @latekvo/add-testing-library-support

ignore type error

c63a561

latekvo marked this pull request as ready for review January 27, 2025 17:40

(amend) update yarn.lock

556f8f2

latekvo requested review from j-piasecki and m-bert January 27, 2025 17:47

j-piasecki reviewed Jan 28, 2025

View reviewed changes

latekvo linked an issue Jan 28, 2025 that may be closed by this pull request

RNGH::Pressable won't work the same as RN::Pressable when using Testing Library #3349

Closed

latekvo added 2 commits January 28, 2025 16:36

change jest check to general test check

f13e7b0

use isTestEnv in pressable

4d942e8

mdjastrzebski mentioned this pull request Jan 28, 2025

feat: testOnly events callstack/react-native-testing-library#1741

Merged

m-bert reviewed Jan 28, 2025

View reviewed changes

latekvo added 2 commits January 28, 2025 17:23

memo isTestEnv

7a31f5e

expose onLongPress

6184078

cache IS_TEST_ENV outside of Pressable

4ec26c4

mdjastrzebski approved these changes Jan 28, 2025

View reviewed changes

sidferreira mentioned this pull request Jan 28, 2025

Improves Pressable's compatibility with RN::Pressable and TestingLibrary #3347

Closed

latekvo requested a review from j-piasecki January 30, 2025 10:44

latekvo requested a review from m-bert January 30, 2025 10:44

m-bert approved these changes Feb 3, 2025

View reviewed changes

latekvo merged commit e36eada into main Feb 3, 2025

latekvo deleted the @latekvo/add-testing-library-support branch February 3, 2025 11:09

	export function isJestEnv(): boolean {
	// @ts-ignore Do not use `@types/node` because it will prioritise Node types over RN types which breaks the types (ex. setTimeout) in React Native projects.
	return hasProperty(global, 'process') && !!process.env.JEST_WORKER_ID;
	}

Uh oh!

Conversation

latekvo commented Jan 27, 2025

Description

Test plan

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latekvo Jan 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski left a comment

Choose a reason for hiding this comment

Uh oh!

latekvo commented Jan 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski commented Jan 28, 2025

Uh oh!

mdjastrzebski commented Jan 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latekvo Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

latekvo commented Jan 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mdjastrzebski left a comment

Choose a reason for hiding this comment

Uh oh!

mdjastrzebski commented Feb 3, 2025

Uh oh!

latekvo commented Feb 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

latekvo Jan 27, 2025 •

edited

Loading

latekvo Jan 28, 2025 •

edited

Loading

latekvo commented Jan 28, 2025 •

edited

Loading