From 87cd9079a31bbde385694bec1a9f095c155f4e9a Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 18 May 2018 11:45:26 -0600 Subject: [PATCH 1/3] upgrade to react 16.3 and convert EuiBasicTable to the new lifecycle --- package.json | 8 +-- src/components/basic_table/basic_table.js | 61 ++++++++++------------- yarn.lock | 60 +++------------------- 3 files changed, 38 insertions(+), 91 deletions(-) diff --git a/package.json b/package.json index 9688c6b3d00..9dcfc988ce0 100644 --- a/package.json +++ b/package.json @@ -110,8 +110,8 @@ "prettier": "^1.11.1", "prop-types": "^15.6.0", "raw-loader": "^0.5.1", - "react": "^16.2.0", - "react-dom": "^16.2.0", + "react": "^16.3.0", + "react-dom": "^16.3.0", "react-redux": "^5.0.6", "react-router": "^3.2.0", "react-router-redux": "^4.0.8", @@ -144,7 +144,7 @@ "peerDependencies": { "moment": "^2.13.0", "prop-types": "^15.5.0", - "react": "^16.2.0 || ^16.2", - "react-dom": "^16.2.0 || ^16.2" + "react": "^16.3", + "react-dom": "^16.3" } } diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js index 57ed4089d6a..62136399a60 100644 --- a/src/components/basic_table/basic_table.js +++ b/src/components/basic_table/basic_table.js @@ -147,6 +147,16 @@ const BasicTablePropTypes = { hasActions: PropTypes.bool }; +export function getItemId(item, props) { + const { itemId } = props; + if (itemId) { + if (isFunction(itemId)) { + return itemId(item); + } + return item[itemId]; + } +} + export class EuiBasicTable extends Component { static propTypes = BasicTablePropTypes; @@ -155,6 +165,19 @@ export class EuiBasicTable extends Component { noItemsMessage: 'No items found', }; + static getDerivedStateFromProps(nextProps, prevState) { + if (!nextProps.selection) { + // next props doesn't have a selection, reset our state + return { selection: [] }; + } + + const selection = prevState.selection.filter(selectedItem => ( + nextProps.items.findIndex(item => getItemId(item, nextProps) === getItemId(selectedItem, nextProps)) !== -1 + )); + + return { selection }; + } + constructor(props) { super(props); this.state = { @@ -177,13 +200,7 @@ export class EuiBasicTable extends Component { } itemId(item) { - const { itemId } = this.props; - if (itemId) { - if (isFunction(itemId)) { - return itemId(item); - } - return item[itemId]; - } + return getItemId(item, this.props); } changeSelection(selection) { @@ -248,30 +265,6 @@ export class EuiBasicTable extends Component { this.props.onChange(criteria); } - // TODO: React 16.3 - getDerivedStateFromProps - componentWillReceiveProps(nextProps) { - // Don't call changeSelection here or else we can get into an infinite loop: - // changeSelection calls props.onSelectionChanged on owner -> - // owner may react by changing props -> - // we receive new props, calling componentWillReceiveProps -> - // and we're in an infinite loop - if (!this.props.selection) { - return; - } - - if (!nextProps.selection) { - this.setState({ selection: [] }); - return; - } - - this.setState(prevState => { - const selection = prevState.selection.filter(selectedItem => ( - nextProps.items.findIndex(item => this.itemId(item) === this.itemId(selectedItem)) !== -1 - )); - return { selection }; - }); - } - render() { const { className, @@ -506,9 +499,9 @@ export class EuiBasicTable extends Component { const cells = []; - const itemId = this.itemId(item) || rowIndex; + const itemId = getItemId(item, this.props) || rowIndex; const selected = !selection ? false : this.state.selection && !!this.state.selection.find(selectedRecord => ( - this.itemId(selectedRecord) === itemId + getItemId(selectedRecord, this.props) === itemId )); if (selection) { @@ -572,7 +565,7 @@ export class EuiBasicTable extends Component { this.changeSelection([...this.state.selection, item]); } else { this.changeSelection(this.state.selection.reduce((selection, selectedItem) => { - if (this.itemId(selectedItem) !== itemId) { + if (getItemId(selectedItem, this.props) !== itemId) { selection.push(selectedItem); } return selection; diff --git a/yarn.lock b/yarn.lock index 969a8d58032..47878d4fe2d 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2888,10 +2888,6 @@ es6-promise@^3.0.2: version "3.3.1" resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-3.3.1.tgz#a08cdde84ccdbf34d027a1451bc91d4bcd28a613" -es6-promise@^4.0.3: - version "4.2.4" - resolved "https://registry.yarnpkg.com/es6-promise/-/es6-promise-4.2.4.tgz#dc4221c2b16518760bd8c39a52d8f356fc00ed29" - es6-set@~0.1.5: version "0.1.5" resolved "https://registry.yarnpkg.com/es6-set/-/es6-set-0.1.5.tgz#d2b3ec5d4d800ced818db538d28974db0a73ccb1" @@ -3681,14 +3677,6 @@ fs-extra@^0.30.0: path-is-absolute "^1.0.0" rimraf "^2.2.8" -fs-extra@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-1.0.0.tgz#cd3ce5f7e7cb6145883fcae3191e9877f8587950" - dependencies: - graceful-fs "^4.1.2" - jsonfile "^2.1.0" - klaw "^1.0.0" - fs-extra@^3.0.1: version "3.0.1" resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-3.0.1.tgz#3794f378c58b342ea7dbbb23095109c4b3b62291" @@ -4212,13 +4200,6 @@ hash.js@^1.0.0, hash.js@^1.0.3: inherits "^2.0.3" minimalistic-assert "^1.0.0" -hasha@^2.2.0: - version "2.2.0" - resolved "https://registry.yarnpkg.com/hasha/-/hasha-2.2.0.tgz#78d7cbfc1e6d66303fe79837365984517b2f6ee1" - dependencies: - is-stream "^1.0.1" - pinkie-promise "^2.0.0" - hawk@3.1.3, hawk@~3.1.3: version "3.1.3" resolved "https://registry.yarnpkg.com/hawk/-/hawk-3.1.3.tgz#078444bd7c1640b0fe540d2c9b73d59678e8e1c4" @@ -7146,20 +7127,6 @@ performance-now@^2.1.0: version "2.1.0" resolved "https://registry.yarnpkg.com/performance-now/-/performance-now-2.1.0.tgz#6309f4e0e5fa913ec1c69307ae364b4b377c9e7b" -phantomjs-prebuilt@^2.1.16: - version "2.1.16" - resolved "https://registry.yarnpkg.com/phantomjs-prebuilt/-/phantomjs-prebuilt-2.1.16.tgz#efd212a4a3966d3647684ea8ba788549be2aefef" - dependencies: - es6-promise "^4.0.3" - extract-zip "^1.6.5" - fs-extra "^1.0.0" - hasha "^2.2.0" - kew "^0.7.0" - progress "^1.1.8" - request "^2.81.0" - request-progress "^2.0.1" - which "^1.2.10" - pify@^2.0.0, pify@^2.3.0: version "2.3.0" resolved "https://registry.yarnpkg.com/pify/-/pify-2.3.0.tgz#ed141a6ac043a849ea588498e7dca8b15330e90c" @@ -7617,10 +7584,6 @@ progress@2.0.0, progress@^2.0.0: version "2.0.0" resolved "https://registry.yarnpkg.com/progress/-/progress-2.0.0.tgz#8a1be366bf8fc23db2bd23f10c6fe920b4389d1f" -progress@^1.1.8: - version "1.1.8" - resolved "https://registry.yarnpkg.com/progress/-/progress-1.1.8.tgz#e260c78f6161cdd9b0e56cc3e0a85de17c7a57be" - promise@^7.1.1: version "7.3.1" resolved "https://registry.yarnpkg.com/promise/-/promise-7.3.1.tgz#064b72602b18f90f29192b8b1bc418ffd1ebd3bf" @@ -7853,9 +7816,9 @@ react-docgen@^2.20.0: node-dir "^0.1.10" recast "^0.12.6" -react-dom@^16.2.0: - version "16.2.0" - resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.2.0.tgz#69003178601c0ca19b709b33a83369fe6124c044" +react-dom@^16.3.0: + version "16.3.2" + resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.3.2.tgz#cb90f107e09536d683d84ed5d4888e9640e0e4df" dependencies: fbjs "^0.8.16" loose-envify "^1.1.0" @@ -7933,9 +7896,9 @@ react-virtualized@^9.18.5: loose-envify "^1.3.0" prop-types "^15.6.0" -react@^16.2.0: - version "16.2.0" - resolved "https://registry.yarnpkg.com/react/-/react-16.2.0.tgz#a31bd2dab89bff65d42134fa187f24d054c273ba" +react@^16.3.0: + version "16.3.2" + resolved "https://registry.yarnpkg.com/react/-/react-16.3.2.tgz#fdc8420398533a1e58872f59091b272ce2f91ea9" dependencies: fbjs "^0.8.16" loose-envify "^1.1.0" @@ -8243,12 +8206,6 @@ replace-ext@^1.0.0: version "1.0.0" resolved "https://registry.yarnpkg.com/replace-ext/-/replace-ext-1.0.0.tgz#de63128373fcbf7c3ccfa4de5a480c45a67958eb" -request-progress@^2.0.1: - version "2.0.1" - resolved "https://registry.yarnpkg.com/request-progress/-/request-progress-2.0.1.tgz#5d36bb57961c673aa5b788dbc8141fdf23b44e08" - dependencies: - throttleit "^1.0.0" - request-promise-core@1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/request-promise-core/-/request-promise-core-1.1.1.tgz#3eee00b2c5aa83239cfb04c5700da36f81cd08b6" @@ -9435,10 +9392,6 @@ throat@^4.0.0: version "4.1.0" resolved "https://registry.yarnpkg.com/throat/-/throat-4.1.0.tgz#89037cbc92c56ab18926e6ba4cbb200e15672a6a" -throttleit@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/throttleit/-/throttleit-1.0.0.tgz#9e785836daf46743145a5984b6268d828528ac6c" - through2@^2.0.0, through2@^2.0.1: version "2.0.3" resolved "https://registry.yarnpkg.com/through2/-/through2-2.0.3.tgz#0004569b37c7c74ba39c43f3ced78d1ad94140be" @@ -10056,6 +10009,7 @@ wdio-visual-regression-service@silne30/wdio-visual-regression-service#Add_Filena lodash "^4.13.1" node-resemble-js "0.0.5" nodeclient-spectre "^1.0.3" + phantomjs-prebuilt "^2.1.16" platform "^1.3.1" wdio-screenshot "^0.6.0" From ef572f4d2550e4a74b9d640a71757c6d7bb31145 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 18 May 2018 11:51:31 -0600 Subject: [PATCH 2/3] added unit tests for EuiBasicTable's getItemId --- .../basic_table/basic_table.test.js | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/components/basic_table/basic_table.test.js b/src/components/basic_table/basic_table.test.js index 7aa0d8bd104..fdc5a9ea905 100644 --- a/src/components/basic_table/basic_table.test.js +++ b/src/components/basic_table/basic_table.test.js @@ -2,7 +2,25 @@ import React from 'react'; import { shallow } from 'enzyme'; import { requiredProps } from '../../test'; -import { EuiBasicTable } from './basic_table'; +import { EuiBasicTable, getItemId } from './basic_table'; + +describe('getItemId', () => { + it('returns undefined if no itemId prop is given', () => { + expect(getItemId({ id: 5 }, {})).toBeUndefined(); + expect(getItemId({ itemId: 5 }, {})).toBeUndefined(); + expect(getItemId({ _itemId: 5 }, {})).toBeUndefined(); + }); + + it('returns the correct id when a string itemId is given', () => { + expect(getItemId({ id: 5 }, { itemId: 'id' })).toBe(5); + expect(getItemId({ thing: '5' }, { itemId: 'thing' })).toBe('5'); + }); + + it('returns the correct id when a function itemId is given', () => { + expect(getItemId({ id: 5 }, { itemId: () => 6 })).toBe(6); + expect(getItemId({ x: 2, y: 4 }, { itemId: ({ x, y }) => x * y })).toBe(8); + }); +}); describe('EuiBasicTable', () => { From 210027d63dbc6dc584d451c672050df4366f96a6 Mon Sep 17 00:00:00 2001 From: Chandler Prall Date: Fri, 18 May 2018 11:54:02 -0600 Subject: [PATCH 3/3] EuiBasicTable#itemId is no longer used --- src/components/basic_table/basic_table.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/components/basic_table/basic_table.js b/src/components/basic_table/basic_table.js index 62136399a60..b61842c3f20 100644 --- a/src/components/basic_table/basic_table.js +++ b/src/components/basic_table/basic_table.js @@ -199,10 +199,6 @@ export class EuiBasicTable extends Component { return criteria; } - itemId(item) { - return getItemId(item, this.props); - } - changeSelection(selection) { if (!this.props.selection) { return;