Skip to content

Commit

Permalink
fix: do not render mocked Modal when visible=false (#39157)
Browse files Browse the repository at this point in the history
Summary:
Note: this PR is related only to testing mocks provided by RN, and does not affect runtime code.

When building new Jest matchers for React Native Testing Library (callstack/react-native-testing-library#1468) I've noticed that when rendering React Native in Jest using React Test Renderer the mocked `Modal` component renders host `Modal` element when `visible={false}`.

This seems to be incorrect as only visible modal should render any host elements. Not visible one should not render any host element even empty ones. Current mock implementation also contradicts the behaviour of non-mocked `Modal` which does not render `RCTModalHostView` in such case.

## Changelog:

[General] [Fixed] - Do not render mocked <Modal /> when `visible=false`

Pull Request resolved: #39157

Test Plan:
I've added test showing that non-mocked Modal renders to `null` and modifies the existing tests so that mocked Modal also renders to `null.`

Luna: I've updated relevant snapshots

Reviewed By: NickGerleman

Differential Revision: D48728277

Pulled By: lunaleaps

fbshipit-source-id: cf06495ad959e2d9549241b57f46f75d7beb9eae
  • Loading branch information
mdjastrzebski authored and facebook-github-bot committed Aug 30, 2023
1 parent 442b852 commit 468a136
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 7 deletions.
15 changes: 13 additions & 2 deletions packages/react-native/Libraries/Modal/__tests__/Modal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ describe('<Modal />', () => {
expect(instance).toMatchSnapshot();
});

it('should not render its children when mocked with visible=false', () => {
it('should not render <Modal> when mocked with visible=false', () => {
const instance = render.create(
<Modal visible={false}>
<View testID="child" />
</Modal>,
);
expect(instance.root.findAllByProps({testID: 'child'})).toHaveLength(0);
expect(instance.toJSON()).toBeNull();
});

it('should shallow render as <Modal> when mocked', () => {
Expand Down Expand Up @@ -65,4 +65,15 @@ describe('<Modal />', () => {
);
expect(instance).toMatchSnapshot();
});

it('should not render <RCTModalHostView> when not mocked with visible=false', () => {
jest.dontMock('../Modal');

const instance = render.create(
<Modal visible={false}>
<View />
</Modal>,
);
expect(instance.toJSON()).toBeNull();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`<Modal /> should render as <RCTModalHostView> when not mocked 1`] = `
<RCTModalHostView
animationType="none"
hardwareAccelerated={false}
identifier={4}
identifier={3}
onDismiss={[Function]}
onStartShouldSetResponder={[Function]}
presentationStyle="fullScreen"
Expand Down
10 changes: 6 additions & 4 deletions packages/react-native/jest/mockModal.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@ import typeof Modal from '../Libraries/Modal/Modal';

function mockModal(BaseComponent: $FlowFixMe) {
class ModalMock extends BaseComponent {
render(): React.Element<Modal> {
render(): React.Element<Modal> | null {
if (this.props.visible === false) {
return null;
}

return (
<BaseComponent {...this.props}>
{this.props.visible !== true ? null : this.props.children}
</BaseComponent>
<BaseComponent {...this.props}>{this.props.children}</BaseComponent>
);
}
}
Expand Down

0 comments on commit 468a136

Please sign in to comment.