Skip to content

Commit

Permalink
Update Modal's mock to not render its children when it is not visible (
Browse files Browse the repository at this point in the history
…#32346)

Summary:
The Modal's mock always render its children (whether it is visible or not), whereas in reality the Modal renders `null` when the Modal is not visible.
This causes troubles when trying to test whether the Modal is visible or not. Instead of testing if the children are rendered (using getByText from React Native Testing Library for instance), we are forced to test the value of the visible prop directly (see callstack/react-native-testing-library#508 and callstack/react-native-testing-library#659).
This is not ideal because we are forced to test implementation detail and can't test from the user perspective. I also believe the mock should be closest as possible from reality.

I had 2 options:
  1. Rendering the Modal without its children
  2. Not rendering the Modal at all

The latter has the advantage of being closer to the reality, but I chose the former to still be able to test the Modal through the visible prop, so there is no breaking change (only snapshots update will be required).

## Changelog

[General] [Changed] - Update Modal's mock to not render its children when it is not visible

Pull Request resolved: #32346

Test Plan:
I added a test case when visible is false, then updated the mock so the children are not rendered. The before / after is here:
![image](https://user-images.githubusercontent.com/17070498/136256142-a351d002-8b77-490a-ba65-1e8ad0d6eb55.png)

Reviewed By: yungsters

Differential Revision: D31445964

Pulled By: lunaleaps

fbshipit-source-id: 08501921455728cde6befd0103016c95074cc1df
  • Loading branch information
AntoineDoubovetzky authored and facebook-github-bot committed Oct 8, 2021
1 parent 27304fc commit ec614c1
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 4 deletions.
9 changes: 9 additions & 0 deletions Libraries/Modal/__tests__/Modal-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,15 @@ describe('<Modal />', () => {
expect(instance).toMatchSnapshot();
});

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

it('should shallow render as <Modal> when mocked', () => {
const output = render.shallow(
<Modal>
Expand Down
2 changes: 1 addition & 1 deletion Libraries/Modal/__tests__/__snapshots__/Modal-test.js.snap
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={1}
identifier={4}
onDismiss={[Function]}
onStartShouldSetResponder={[Function]}
presentationStyle="fullScreen"
Expand Down
31 changes: 31 additions & 0 deletions jest/mockModal.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @format
* @flow strict-local
*/

/* eslint-env jest */

'use strict';

const React = require('react');
const Modal = require('../Libraries/Modal/Modal');

function mockModal(BaseComponent: $FlowFixMe) {
class ModalMock extends BaseComponent {
render(): React.Element<typeof Modal> {
return (
<BaseComponent {...this.props}>
{this.props.visible !== true ? null : this.props.children}
</BaseComponent>
);
}
}
return ModalMock;
}

module.exports = (mockModal: $FlowFixMe);
8 changes: 5 additions & 3 deletions jest/setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,11 @@ jest
getNativeRef: jest.fn(),
}),
)
.mock('../Libraries/Modal/Modal', () =>
mockComponent('../Libraries/Modal/Modal'),
)
.mock('../Libraries/Modal/Modal', () => {
const baseComponent = mockComponent('../Libraries/Modal/Modal');
const mockModal = jest.requireActual('./mockModal');
return mockModal(baseComponent);
})
.mock('../Libraries/Components/View/View', () =>
mockComponent('../Libraries/Components/View/View', MockNativeMethods),
)
Expand Down

0 comments on commit ec614c1

Please sign in to comment.