From 5e06ca13c3bd8212bd901294719ace57d6c3b55c Mon Sep 17 00:00:00 2001 From: Chandler Date: Wed, 16 May 2018 08:58:24 -0600 Subject: [PATCH] Move EuiBasicTable's itemId prop out of selection into a top-level prop (#830) * Move EuiBasicTable's itemId prop out of selection into a top-level prop * changelog * add generic proptype validator for associating properties * added unit test for withRequiredProp * re-add deleted proptype, commit with_required_prop's snapshots * update changelog * tweaks in response to PR feedback --- CHANGELOG.md | 7 +- src-docs/src/views/tables/actions/actions.js | 2 +- src-docs/src/views/tables/basic/props_info.js | 10 +-- .../tables/expanding_rows/expanding_rows.js | 2 +- .../tables/in_memory/in_memory_selection.js | 2 +- src-docs/src/views/tables/mobile/mobile.js | 2 +- .../src/views/tables/selection/selection.js | 2 +- src/components/badge/badge.js | 16 +++- .../__snapshots__/basic_table.test.js.snap | 34 ++++---- .../in_memory_table.test.js.snap | 29 ++----- .../basic_table/basic_table.behavior.test.js | 2 +- src/components/basic_table/basic_table.js | 23 +++--- .../basic_table/basic_table.test.js | 15 ++-- src/components/basic_table/in_memory_table.js | 6 +- .../basic_table/in_memory_table.test.js | 13 +-- .../with_required_prop.test.js.snap | 7 ++ src/utils/prop_types/index.js | 4 +- src/utils/prop_types/requires_aria_label.js | 27 ------- src/utils/prop_types/with_required_prop.js | 35 ++++++++ .../prop_types/with_required_prop.test.js | 79 +++++++++++++++++++ 20 files changed, 207 insertions(+), 110 deletions(-) create mode 100644 src/utils/prop_types/__snapshots__/with_required_prop.test.js.snap delete mode 100644 src/utils/prop_types/requires_aria_label.js create mode 100644 src/utils/prop_types/with_required_prop.js create mode 100644 src/utils/prop_types/with_required_prop.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 0d419adc94c..5156fd31ab9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -No public interface changes since `0.0.46`. +**Breaking changes** + +- Move `EuiBasicTable`'s `itemId` prop from `selection` to a top-level property ([#830](https://github.com/elastic/eui/pull/830)) +- Renamed/refactored `requiresAriaLabel` prop validator to a more general `withRequiredProp` ([#830](https://github.com/elastic/eui/pull/830)) ## [`0.0.47`](https://github.com/elastic/eui/tree/v0.0.47) @@ -20,7 +23,7 @@ No public interface changes since `0.0.46`. - Made boolean matching in `EuiSearchBar` more exact so it doesn't match words starting with booleans, like "truest" or "offer" ([#776](https://github.com/elastic/eui/pull/776)) - `EuiComboBox` do not setState or call refs once component is unmounted ([807](https://github.com/elastic/eui/pull/807) and [#813](https://github.com/elastic/eui/pull/813)) - Added better accessibility labeling to `EuiPagination`, `EuiSideNav`, `EuiPopover`, `EuiBottomBar` and `EuiBasicTable`. ([#821](https://github.com/elastic/eui/pull/821)) -- Added `isDisabled` to `EuiComboBox` ([#829](https://github.com/elastic/eui/pull/829)) +- Added `isDisabled` to `EuiComboBox` ([#829](https://github.com/elastic/eui/pull/829)) ## [`0.0.46`](https://github.com/elastic/eui/tree/v0.0.46) diff --git a/src-docs/src/views/tables/actions/actions.js b/src-docs/src/views/tables/actions/actions.js index c622eea21cd..f4d868f2126 100644 --- a/src-docs/src/views/tables/actions/actions.js +++ b/src-docs/src/views/tables/actions/actions.js @@ -262,7 +262,6 @@ export class Table extends Component { }; const selection = { - itemId: 'id', selectable: (user) => user.online, selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined, onSelectionChange: this.onSelectionChange @@ -292,6 +291,7 @@ export class Table extends Component { string' } + }, compressed: { description: 'Makes the font and padding smaller for the entire table', type: { name: 'bool' } @@ -88,11 +93,6 @@ export const propsInfo = { __docgenInfo: { _euiObjectType: 'type', props: { - itemId: { - description: 'describes how to extract a unique ID from each item', - required: true, - type: { name: 'string | (item) => string' } - }, onSelectionChanged: { description: 'A callback that will be called whenever the item selection changes', required: false, diff --git a/src-docs/src/views/tables/expanding_rows/expanding_rows.js b/src-docs/src/views/tables/expanding_rows/expanding_rows.js index fcae31a3546..5dd89ee47c9 100644 --- a/src-docs/src/views/tables/expanding_rows/expanding_rows.js +++ b/src-docs/src/views/tables/expanding_rows/expanding_rows.js @@ -208,7 +208,6 @@ export class Table extends Component { }; const selection = { - itemId: 'id', selectable: (user) => user.online, selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined, onSelectionChange: this.onSelectionChange @@ -219,6 +218,7 @@ export class Table extends Component { {deleteButton} user.online, selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined, onSelectionChange: (selection) => this.setState({ selection }) @@ -227,6 +226,7 @@ export class Table extends Component {
user.online, selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined, onSelectionChange: this.onSelectionChange @@ -219,6 +218,7 @@ export class Table extends Component { user.online, selectableMessage: (selectable) => !selectable ? 'User is currently offline' : undefined, onSelectionChange: this.onSelectionChange @@ -203,6 +202,7 @@ export class Table extends Component { {deleteButton} - - - - name1 - - - @@ -234,7 +218,7 @@ exports[`EuiBasicTable itemIdToExpandedRowMap renders an expanded row 1`] = ` key="_data_column_name_1_0" textOnly={true} > - name2 + name1 + name2 + + + + + + name3 diff --git a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap index e628f608a7b..5a5e03bfbe2 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -11,7 +11,6 @@ exports[`EuiInMemoryTable empty array 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={Array []} noItemsMessage="No items found" onChange={[Function]} @@ -31,7 +30,6 @@ exports[`EuiInMemoryTable with initial sorting 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -73,7 +71,6 @@ exports[`EuiInMemoryTable with items 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -107,6 +104,7 @@ exports[`EuiInMemoryTable with items and expanded item 1`] = ` }, ] } + itemId="id" itemIdToExpandedRowMap={ Object { "1":
@@ -147,7 +145,6 @@ exports[`EuiInMemoryTable with items and message - expecting to show the items 1 }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -181,7 +178,6 @@ exports[`EuiInMemoryTable with message 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={Array []} noItemsMessage="where my items at?" onChange={[Function]} @@ -200,7 +196,6 @@ exports[`EuiInMemoryTable with message and loading 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={Array []} loading={true} noItemsMessage="Loading items...." @@ -220,7 +215,6 @@ exports[`EuiInMemoryTable with pagination 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -262,7 +256,6 @@ exports[`EuiInMemoryTable with pagination and default page size 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -308,7 +301,7 @@ exports[`EuiInMemoryTable with pagination and selection 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -342,7 +335,6 @@ exports[`EuiInMemoryTable with pagination and selection 1`] = ` responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -361,7 +353,6 @@ exports[`EuiInMemoryTable with pagination, default page size and error 1`] = ` ] } error="ouch!" - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { @@ -400,7 +391,7 @@ exports[`EuiInMemoryTable with pagination, selection and sorting 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -434,7 +425,6 @@ exports[`EuiInMemoryTable with pagination, selection and sorting 1`] = ` responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -476,7 +466,7 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and simple search }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -510,7 +500,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and simple search responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -546,7 +535,7 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and a single recor }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -580,7 +569,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and a single recor responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -605,7 +593,7 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and column rendere }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -635,7 +623,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and column rendere responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -697,7 +684,7 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and configured sea }, ] } - itemIdToExpandedRowMap={Object {}} + itemId="id" items={ Array [ Object { @@ -723,7 +710,6 @@ exports[`EuiInMemoryTable with pagination, selection, sorting and configured sea responsive={true} selection={ Object { - "itemId": "id", "onSelectionChanged": [Function], } } @@ -748,7 +734,6 @@ exports[`EuiInMemoryTable with sorting 1`] = ` }, ] } - itemIdToExpandedRowMap={Object {}} items={ Array [ Object { diff --git a/src/components/basic_table/basic_table.behavior.test.js b/src/components/basic_table/basic_table.behavior.test.js index 3915c430176..8cb529df852 100644 --- a/src/components/basic_table/basic_table.behavior.test.js +++ b/src/components/basic_table/basic_table.behavior.test.js @@ -16,6 +16,7 @@ describe('EuiBasicTable', () => { { id: '1', name: 'name1' }, { id: '2', name: 'name2' } ], + itemId: 'id', columns: [ { field: 'name', @@ -24,7 +25,6 @@ describe('EuiBasicTable', () => { } ], selection: { - itemId: 'id', onSelectionChanged: () => {} }, onChange: () => {} diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js index 1582bf3ee08..95fa58924f7 100644 --- a/src/components/basic_table/basic_table.js +++ b/src/components/basic_table/basic_table.js @@ -28,6 +28,7 @@ import { EuiIcon } from '../icon/icon'; import { LoadingTableBody } from './loading_table_body'; import { EuiTableHeaderMobile } from '../table/mobile/table_header_mobile'; import { EuiTableSortMobile } from '../table/mobile/table_sort_mobile'; +import { withRequiredProp } from '../../utils/prop_types/with_required_prop'; const dataTypesProfiles = { auto: { @@ -111,13 +112,12 @@ export const ComputedColumnType = PropTypes.shape({ export const ColumnType = PropTypes.oneOfType([FieldDataColumnType, ComputedColumnType, ActionsColumnType]); -const ItemIdType = PropTypes.oneOfType([ +export const ItemIdType = PropTypes.oneOfType([ PropTypes.string, // the name of the item id property PropTypes.func // (item) => string ]); export const SelectionType = PropTypes.shape({ - itemId: ItemIdType.isRequired, onSelectionChange: PropTypes.func, // (selection: Record[]) => void;, selectable: PropTypes.func, // (item) => boolean; selectableMessage: PropTypes.func // (selectable, item) => boolean; @@ -129,16 +129,18 @@ const SortingType = PropTypes.shape({ const BasicTablePropTypes = { items: PropTypes.array.isRequired, + itemId: ItemIdType, columns: PropTypes.arrayOf(ColumnType).isRequired, pagination: PaginationType, sorting: SortingType, - selection: SelectionType, + selection: withRequiredProp(SelectionType, 'itemId', 'row selection uses the itemId prop to identify each row'), onChange: PropTypes.func, error: PropTypes.string, loading: PropTypes.bool, noItemsMessage: PropTypes.node, className: PropTypes.string, compressed: PropTypes.bool, + itemIdToExpandedRowMap: withRequiredProp(PropTypes.object, 'itemId', 'row expansion uses the itemId prop to identify each row') }; export class EuiBasicTable extends Component { @@ -147,7 +149,6 @@ export class EuiBasicTable extends Component { static defaultProps = { responsive: true, noItemsMessage: 'No items found', - itemIdToExpandedRowMap: {}, }; constructor(props) { @@ -172,12 +173,12 @@ export class EuiBasicTable extends Component { } itemId(item) { - const { selection } = this.props; - if (selection) { - if (isFunction(selection.itemId)) { - return selection.itemId(item); + const { itemId } = this.props; + if (itemId) { + if (isFunction(itemId)) { + return itemId(item); } - return item[selection.itemId]; + return item[itemId]; } } @@ -477,11 +478,11 @@ export class EuiBasicTable extends Component { } renderItemRow(item, rowIndex) { - const { columns, selection, isSelectable, hasActions, itemIdToExpandedRowMap, isExpandable } = this.props; + const { columns, selection, isSelectable, hasActions, itemIdToExpandedRowMap = {}, isExpandable } = this.props; const cells = []; - const itemId = selection ? this.itemId(item) : rowIndex; + const itemId = this.itemId(item) || rowIndex; const selected = !selection ? false : this.state.selection && !!this.state.selection.find(selectedRecord => ( this.itemId(selectedRecord) === itemId )); diff --git a/src/components/basic_table/basic_table.test.js b/src/components/basic_table/basic_table.test.js index 53e27695f96..7aa0d8bd104 100644 --- a/src/components/basic_table/basic_table.test.js +++ b/src/components/basic_table/basic_table.test.js @@ -99,6 +99,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -272,6 +273,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -285,7 +287,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, onChange: () => {} @@ -306,6 +307,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -320,7 +322,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { @@ -344,6 +345,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -359,7 +361,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { @@ -383,6 +384,7 @@ describe('EuiBasicTable', () => { { id: '2', count: 2 }, { id: '3', count: 3 } ], + itemId: 'id', columns: [ { field: 'count', @@ -398,7 +400,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { @@ -423,6 +424,7 @@ describe('EuiBasicTable', () => { { id: '2', count: 2 }, { id: '3', count: 3 } ], + itemId: 'id', columns: [ { field: 'count', @@ -439,7 +441,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { @@ -463,6 +464,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -488,7 +490,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { @@ -512,6 +513,7 @@ describe('EuiBasicTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -543,7 +545,6 @@ describe('EuiBasicTable', () => { totalItemCount: 5 }, selection: { - itemId: 'id', onSelectionChanged: () => undefined }, sorting: { diff --git a/src/components/basic_table/in_memory_table.js b/src/components/basic_table/in_memory_table.js index d06bf0f05da..ba851cac69d 100644 --- a/src/components/basic_table/in_memory_table.js +++ b/src/components/basic_table/in_memory_table.js @@ -4,6 +4,7 @@ import { EuiBasicTable, ColumnType, SelectionType, + ItemIdType, } from './basic_table'; import { defaults as paginationBarDefaults @@ -54,7 +55,8 @@ const InMemoryTablePropTypes = { sort: PropertySortType }) ]), - selection: SelectionType + selection: SelectionType, + itemId: ItemIdType }; const getInitialQuery = (search) => { @@ -260,6 +262,7 @@ export class EuiInMemoryTable extends Component { pagination: hasPagination, sorting: hasSorting, itemIdToExpandedRowMap, + itemId, } = this.props; const { @@ -295,6 +298,7 @@ export class EuiInMemoryTable extends Component { const table = ( { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -293,6 +294,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -302,7 +304,6 @@ describe('EuiInMemoryTable', () => { ], pagination: true, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; @@ -322,6 +323,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -333,7 +335,6 @@ describe('EuiInMemoryTable', () => { pagination: true, sorting: true, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; @@ -353,6 +354,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -367,7 +369,6 @@ describe('EuiInMemoryTable', () => { }, sorting: true, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; @@ -387,6 +388,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -409,7 +411,6 @@ describe('EuiInMemoryTable', () => { pagination: true, sorting: true, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; @@ -429,6 +430,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -452,7 +454,6 @@ describe('EuiInMemoryTable', () => { sorting: true, search: true, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; @@ -472,6 +473,7 @@ describe('EuiInMemoryTable', () => { { id: '2', name: 'name2' }, { id: '3', name: 'name3' } ], + itemId: 'id', columns: [ { field: 'name', @@ -510,7 +512,6 @@ describe('EuiInMemoryTable', () => { ] }, selection: { - itemId: 'id', onSelectionChanged: () => undefined } }; diff --git a/src/utils/prop_types/__snapshots__/with_required_prop.test.js.snap b/src/utils/prop_types/__snapshots__/with_required_prop.test.js.snap new file mode 100644 index 00000000000..8edc8511cd2 --- /dev/null +++ b/src/utils/prop_types/__snapshots__/with_required_prop.test.js.snap @@ -0,0 +1,7 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`withRequiredProp warns when the base prop is present and valid but the required prop is missing 1`] = `"Warning: Failed exampleProp type: Property \\"exampleProp\\" was passed without corresponding property \\"requiredProp\\""`; + +exports[`withRequiredProp warns when the underlying prop validator fails 1`] = `"Warning: Failed exampleProp type: Invalid exampleProp \`exampleProp\` of type \`number\` supplied to \`ExampleComponent\`, expected \`string\`."`; + +exports[`withRequiredProp warns with a custom message when validation fails 1`] = `"Warning: Failed exampleProp type: Property \\"exampleProp\\" was passed without corresponding property \\"requiredProp\\"; a custom message"`; diff --git a/src/utils/prop_types/index.js b/src/utils/prop_types/index.js index 43a2fd6873a..85275d1e846 100644 --- a/src/utils/prop_types/index.js +++ b/src/utils/prop_types/index.js @@ -1,7 +1,7 @@ import { is } from './is'; -import { requiresAriaLabel } from './requires_aria_label'; +import { withRequiredProp } from './with_required_prop'; export const EuiPropTypes = { is, - requiresAriaLabel, + withRequiredProp, }; diff --git a/src/utils/prop_types/requires_aria_label.js b/src/utils/prop_types/requires_aria_label.js deleted file mode 100644 index 835cd91518e..00000000000 --- a/src/utils/prop_types/requires_aria_label.js +++ /dev/null @@ -1,27 +0,0 @@ -/** - * PropType validation to require an aria label if the associated function property is also present - * - * example: - * ExampleComponent.propTypes = { - * onClick: PropTypes.func, - * onClickAriaLabel: requiresAriaLabel('onClick') - * } - * - * this validator warns if ExampleComponent is passed an `onClick` prop with no `onClickAriaLabel` - */ -export const requiresAriaLabel = (action) => { - - const validator = (props, propName, componentName) => { - // if the associated action property exists but the aria label property is missing - if (props[action] && !props[propName]) { - return new Error( - `Please provide an aria label to compliment your ${action} ` + - `prop in ${componentName}` - ); - } - - return null; - }; - - return validator; -}; diff --git a/src/utils/prop_types/with_required_prop.js b/src/utils/prop_types/with_required_prop.js new file mode 100644 index 00000000000..f1dba878815 --- /dev/null +++ b/src/utils/prop_types/with_required_prop.js @@ -0,0 +1,35 @@ +/** + * PropType validation that, if the property is present, + * validates against a proptype and verifies that another property exists + * + * example: + * ExampleComponent.propTypes = { + * items: PropTypes.array, + * itemId: withRequiredProp(PropTypes.string, 'items', 'itemId is required to extract the ID from an item') + * } + * + * this validator warns if ExampleComponent is passed an `items` prop but not `itemId` + */ +export const withRequiredProp = (proptype, requiredPropName, messageDescription) => { + const validator = (...args) => { + const [props, propName] = args; + + // run the proptype for this property + let result = proptype(...args); + + // if the property type checking passed then check for the required prop + if (result == null) { + // if this property was passed, check that the required property also exists + if (props[propName] != null && props[requiredPropName] == null) { + result = new Error( + `Property "${propName}" was passed without corresponding property "${requiredPropName}"` + + (messageDescription ? `; ${messageDescription}` : '') + ); + } + } + + return result; + }; + + return validator; +}; diff --git a/src/utils/prop_types/with_required_prop.test.js b/src/utils/prop_types/with_required_prop.test.js new file mode 100644 index 00000000000..71bc9b1b469 --- /dev/null +++ b/src/utils/prop_types/with_required_prop.test.js @@ -0,0 +1,79 @@ +import { withRequiredProp } from './with_required_prop'; +import PropTypes from 'prop-types'; + +describe('withRequiredProp', () => { + it('warns when the underlying prop validator fails', () => { + expect(() => { + PropTypes.checkPropTypes( + { + exampleProp: withRequiredProp(PropTypes.string, 'requiredProp') + }, + { + exampleProp: 15 + }, + 'exampleProp', + 'ExampleComponent' + ); + }).toThrowErrorMatchingSnapshot(); + }); + + it('warns when the base prop is present and valid but the required prop is missing', () => { + expect(() => { + PropTypes.checkPropTypes( + { + exampleProp: withRequiredProp(PropTypes.string, 'requiredProp') + }, + { + exampleProp: 'hello' + }, + 'exampleProp', + 'ExampleComponent' + ); + }).toThrowErrorMatchingSnapshot(); + }); + + it('warns with a custom message when validation fails', () => { + expect(() => { + PropTypes.checkPropTypes( + { + exampleProp: withRequiredProp(PropTypes.string, 'requiredProp', 'a custom message') + }, + { + exampleProp: 'hello' + }, + 'exampleProp', + 'ExampleComponent' + ); + }).toThrowErrorMatchingSnapshot(); + }); + + it('does not warn when the base property is missing', () => { + expect(() => { + PropTypes.checkPropTypes( + { + exampleProp: withRequiredProp(PropTypes.string, 'requiredProp') + }, + {}, + 'exampleProp', + 'ExampleComponent' + ); + }).not.toThrow(); + }); + + it('does not warn when both the base property and required properties exist', () => { + expect(() => { + PropTypes.checkPropTypes( + { + exampleProp: withRequiredProp(PropTypes.string, 'requiredProp'), + requiredProp: PropTypes.number + }, + { + exampleProp: 'hello', + requiredProp: 5 + }, + 'exampleProp', + 'ExampleComponent' + ); + }).not.toThrow(); + }); +});