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

Add testing-library compatibility props #3357

Open
wants to merge 12 commits into
base: main
Choose a base branch
from
Open

Add testing-library compatibility props #3357

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
122d977
add testOnly onPressX props to Pressable
latekvo Jan 27, 2025
e51fdcd
update test env detection
latekvo Jan 27, 2025
9828131
Merge branch 'main' into @latekvo/add-testing-library-support
latekvo Jan 27, 2025
930dabc
add node type defs
latekvo Jan 27, 2025
f1ac3ce
Merge branch '@latekvo/add-testing-library-support' of https://github…
latekvo Jan 27, 2025
c63a561
ignore type error
latekvo Jan 27, 2025
556f8f2
(amend) update yarn.lock
latekvo Jan 27, 2025
f13e7b0
change jest check to general test check
latekvo Jan 28, 2025
4d942e8
use isTestEnv in pressable
latekvo Jan 28, 2025
7a31f5e
memo isTestEnv
latekvo Jan 28, 2025
6184078
expose onLongPress
latekvo Jan 28, 2025
4ec26c4
cache IS_TEST_ENV outside of Pressable
latekvo Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/components/GestureButtonsProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,24 @@ export interface RawButtonProps
* Style object, use it to set additional styles.
*/
style?: StyleProp<ViewStyle>;

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

Choose a reason for hiding this comment

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

What kind of argument should be passed to testOnly_onPress?

Copy link
Contributor Author

@latekvo latekvo Jan 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

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);

  [...]
}


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

/**
* Used for testing-library compatibility, not passed to the native component.
*/
// eslint-disable-next-line @typescript-eslint/ban-types
testOnly_onPressOut?: Function | null;
}
interface ButtonWithRefProps {
innerRef?: React.ForwardedRef<React.ComponentType<any>>;
Expand Down
7 changes: 6 additions & 1 deletion src/components/Pressable/Pressable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,6 +380,8 @@ export default function Pressable(props: PressableProps) {
? children({ pressed: pressedState })
: children;

const inJestEnv = typeof jest !== 'undefined';
Copy link

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in e51fdcd

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link

Choose a reason for hiding this comment

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

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


return (
<GestureDetector gesture={gesture}>
<NativeButton
Expand All @@ -390,7 +392,10 @@ export default function Pressable(props: PressableProps) {
touchSoundDisabled={android_disableSound ?? undefined}
rippleColor={processColor(android_ripple?.color ?? defaultRippleColor)}
rippleRadius={android_ripple?.radius ?? undefined}
style={[pointerStyle, styleProp]}>
style={[pointerStyle, styleProp]}
testOnly_onPress={inJestEnv ? onPress : undefined}
testOnly_onPressIn={inJestEnv ? onPressIn : undefined}
testOnly_onPressOut={inJestEnv ? onPressOut : undefined}>
{childrenProp}
{__DEV__ ? (
<PressabilityDebugView color="red" hitSlop={normalizedHitSlop} />
Expand Down
Loading