Skip to content

Commit

Permalink
Upgrade to React 16.3 and convert EuiBasicTable's lifecycle (#849)
Browse files Browse the repository at this point in the history 
* upgrade to react 16.3 and convert EuiBasicTable to the new lifecycle

* added unit tests for EuiBasicTable's getItemId

* EuiBasicTable#itemId is no longer used
  • Loading branch information
@chandlerprall
chandlerprall authored May 18, 2018
1 parent 43e0d68 commit ae71b41
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 95 deletions.
8 changes: 4 additions & 4 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
}
63 changes: 26 additions & 37 deletions src/components/basic_table/basic_table.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = {
Expand All @@ -176,16 +199,6 @@ export class EuiBasicTable extends Component {
return criteria;
}

itemId(item) {
const { itemId } = this.props;
if (itemId) {
if (isFunction(itemId)) {
return itemId(item);
}
return item[itemId];
}
}

changeSelection(selection) {
if (!this.props.selection) {
return;
Expand Down Expand Up @@ -248,30 +261,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,
Expand Down Expand Up @@ -506,9 +495,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) {
Expand Down Expand Up @@ -572,7 +561,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;
Expand Down
20 changes: 19 additions & 1 deletion src/components/basic_table/basic_table.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {

Expand Down
60 changes: 7 additions & 53 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"

[email protected], hawk@~3.1.3:
version "3.1.3"
resolved "https://registry.yarnpkg.com/hawk/-/hawk-3.1.3.tgz#078444bd7c1640b0fe540d2c9b73d59678e8e1c4"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -7617,10 +7584,6 @@ [email protected], 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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"

[email protected]:
version "1.1.1"
resolved "https://registry.yarnpkg.com/request-promise-core/-/request-promise-core-1.1.1.tgz#3eee00b2c5aa83239cfb04c5700da36f81cd08b6"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"

Expand Down

0 comments on commit ae71b41

Please sign in to comment.