Skip to content

Commit 7c22461

Browse files
Tom Colemanwuweiweiwu
authored andcommitted
Use getDerivedStateFromProps more simply (#315)
* Update to React@16.4 and add a test for strict mode This test is a reproduction for #309 and so is currently failing. There is some complexity in making it work but it reproduces the problem. * Don't try and avoid updating state I'm not sure if this is the best solution for #309, as I am not entirely aware of what this code was trying to do, but it *seemed* like it was trying to avoid touching state too much which doesn't seem super useful as there wasn't any complex calculations involved -- and if there was, memoization would be more useful. In any case it was triggering problems in strict mode which implies it would have broken in later version of React.
1 parent 8c4fdcc commit 7c22461

File tree

5 files changed

+53
-71
lines changed

5 files changed

+53
-71
lines changed

package.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@
6262
"mochify": "^5.8.0",
6363
"mochify-istanbul": "^2.4.2",
6464
"prettier": "^1.13.5",
65-
"react": "^15.6.1",
66-
"react-dom": "^15.6.1",
65+
"react": "^16.4.2",
66+
"react-dom": "^16.4.2",
6767
"rimraf": "^2.6.2",
6868
"rollup": "^0.60.7",
6969
"rollup-plugin-babel": "^3.0.4",

src/SplitPane.js

Lines changed: 5 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ function getDefaultSize(defaultSize, minSize, maxSize, draggedSize) {
3737
}
3838

3939
function removeNullChildren(children) {
40-
return React.Children.toArray(children).filter(c => c);
40+
return React.Children.toArray(children).filter(c => c);
4141
}
4242
class SplitPane extends React.Component {
4343
constructor(props) {
@@ -65,15 +65,6 @@ class SplitPane extends React.Component {
6565
resized: false,
6666
pane1Size: primary === 'first' ? initialSize : undefined,
6767
pane2Size: primary === 'second' ? initialSize : undefined,
68-
69-
// previous props that we need in static methods
70-
instanceProps: {
71-
primary,
72-
size,
73-
defaultSize,
74-
minSize,
75-
maxSize,
76-
},
7768
};
7869
}
7970

@@ -214,7 +205,6 @@ class SplitPane extends React.Component {
214205
// TODO: find a more elegant way to fix this. memoize calls to setSize?
215206
// we have to check values since gDSFP is called on every render
216207
static setSize(props, state) {
217-
const { instanceProps } = state;
218208
const newState = {};
219209

220210
const newSize =
@@ -227,43 +217,14 @@ class SplitPane extends React.Component {
227217
state.draggedSize
228218
);
229219

230-
const defaultSizeChanged =
231-
props.defaultSize !== instanceProps.defaultSize ||
232-
props.minSize !== instanceProps.minSize ||
233-
props.maxSize !== instanceProps.maxSize;
234-
235-
const shouldUpdateSize =
236-
props.size !== undefined
237-
? props.size !== instanceProps.size
238-
: defaultSizeChanged;
239-
240-
if (
241-
props.size !== undefined &&
242-
props.size !== state.draggedSize &&
243-
shouldUpdateSize
244-
) {
220+
if (props.size !== undefined) {
245221
newState.draggedSize = newSize;
246222
}
247223

248224
const isPanel1Primary = props.primary === 'first';
249225

250-
if (shouldUpdateSize || props.primary !== state.instanceProps.primary) {
251-
newState[isPanel1Primary ? 'pane1Size' : 'pane2Size'] = newSize;
252-
}
253-
254-
// unset the size on the non primary panel
255-
if (props.primary !== state.instanceProps.primary) {
256-
newState[isPanel1Primary ? 'pane2Size' : 'pane1Size'] = undefined;
257-
}
258-
259-
// update the values in instanceProps
260-
instanceProps.primary = props.primary;
261-
instanceProps.size = props.size;
262-
instanceProps.defaultSize = props.defaultSize;
263-
instanceProps.minSize = props.minSize;
264-
instanceProps.maxSize = props.maxSize;
265-
266-
newState.instanceProps = instanceProps;
226+
newState[isPanel1Primary ? 'pane1Size' : 'pane2Size'] = newSize;
227+
newState[isPanel1Primary ? 'pane2Size' : 'pane1Size'] = undefined;
267228

268229
return newState;
269230
}
@@ -296,7 +257,7 @@ class SplitPane extends React.Component {
296257
: resizerClassName;
297258

298259
const notNullChildren = removeNullChildren(children);
299-
260+
300261
const style = Object.assign(
301262
{},
302263
{

test/assertions/Asserter.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,5 +217,10 @@ export default (jsx, renderToDom = false) => {
217217
expect(primary.props.size).to.equal(undefined);
218218
expect(secondary.props.size).to.equal(50);
219219
},
220+
221+
assertPaneWidthChange(newJsx, expectedWidth, pane = 'first') {
222+
updateComponent(newJsx);
223+
return assertPaneStyles({ width: expectedWidth }, pane);
224+
},
220225
};
221226
};

test/default-split-pane-tests.js

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React from 'react';
1+
import React, { StrictMode } from 'react';
22
import chai from 'chai';
33
import spies from 'chai-spies';
44

@@ -145,7 +145,6 @@ describe('Component updates', () => {
145145
<div>two</div>
146146
</SplitPane>
147147
);
148-
149148
it('unsets the width on the non-primary panel when first', () => {
150149
asserter(splitPane1).assertPrimaryPanelChange(
151150
splitPane2,
@@ -161,4 +160,30 @@ describe('Component updates', () => {
161160
'first'
162161
);
163162
});
163+
164+
it('updates the width of first panel when updating size, in strict mode (#309)', () => {
165+
// For some reason StrictMode renders to null if it is the root of the jsx,
166+
// and we also need the root to be a class-based component. So this is just a complicated
167+
// way of getting around this problem.
168+
class Div extends React.Component {
169+
render() {
170+
return <div>{this.props.children}</div>;
171+
}
172+
}
173+
const paneWithWidth = size => (
174+
<Div>
175+
<StrictMode>
176+
<SplitPane primary="first" size={size}>
177+
<div>one</div>
178+
<div>two</div>
179+
</SplitPane>
180+
</StrictMode>
181+
</Div>
182+
);
183+
184+
asserter(paneWithWidth(100)).assertPaneWidthChange(
185+
paneWithWidth(200),
186+
'200px'
187+
);
188+
});
164189
});

yarn.lock

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1809,14 +1809,6 @@ create-hmac@^1.1.0, create-hmac@^1.1.2, create-hmac@^1.1.4:
18091809
safe-buffer "^5.0.1"
18101810
sha.js "^2.4.8"
18111811

1812-
create-react-class@^15.6.0:
1813-
version "15.6.3"
1814-
resolved "https://registry.yarnpkg.com/create-react-class/-/create-react-class-15.6.3.tgz#2d73237fb3f970ae6ebe011a9e66f46dbca80036"
1815-
dependencies:
1816-
fbjs "^0.8.9"
1817-
loose-envify "^1.3.1"
1818-
object-assign "^4.1.1"
1819-
18201812
cross-env@^5.2.0:
18211813
version "5.2.0"
18221814
resolved "https://registry.yarnpkg.com/cross-env/-/cross-env-5.2.0.tgz#6ecd4c015d5773e614039ee529076669b9d126f2"
@@ -2470,7 +2462,7 @@ fast-levenshtein@~2.0.4:
24702462
version "2.0.6"
24712463
resolved "https://registry.yarnpkg.com/fast-levenshtein/-/fast-levenshtein-2.0.6.tgz#3d8a5c66883a16a30ca8643e851f19baa7797917"
24722464

2473-
fbjs@^0.8.9:
2465+
fbjs@^0.8.16:
24742466
version "0.8.17"
24752467
resolved "https://registry.yarnpkg.com/fbjs/-/fbjs-0.8.17.tgz#c4d598ead6949112653d6588b01a5cdcd9f90fdd"
24762468
dependencies:
@@ -4387,7 +4379,7 @@ prompt@~0.2.14:
43874379
utile "0.2.x"
43884380
winston "0.8.x"
43894381

4390-
prop-types@^15.5.10, prop-types@^15.5.4, prop-types@^15.6.1:
4382+
prop-types@^15.5.10, prop-types@^15.5.4, prop-types@^15.6.0, prop-types@^15.6.1:
43914383
version "15.6.2"
43924384
resolved "https://registry.yarnpkg.com/prop-types/-/prop-types-15.6.2.tgz#05d5ca77b4453e985d60fc7ff8c859094a497102"
43934385
dependencies:
@@ -4483,14 +4475,14 @@ rc@^1.1.7:
44834475
minimist "^1.2.0"
44844476
strip-json-comments "~2.0.1"
44854477

4486-
react-dom@^15.6.1:
4487-
version "15.6.2"
4488-
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-15.6.2.tgz#41cfadf693b757faf2708443a1d1fd5a02bef730"
4478+
react-dom@^16.4.2:
4479+
version "16.4.2"
4480+
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.4.2.tgz#4afed569689f2c561d2b8da0b819669c38a0bda4"
44894481
dependencies:
4490-
fbjs "^0.8.9"
4482+
fbjs "^0.8.16"
44914483
loose-envify "^1.1.0"
4492-
object-assign "^4.1.0"
4493-
prop-types "^15.5.10"
4484+
object-assign "^4.1.1"
4485+
prop-types "^15.6.0"
44944486

44954487
react-lifecycles-compat@^3.0.4:
44964488
version "3.0.4"
@@ -4502,15 +4494,14 @@ react-style-proptype@^3.0.0:
45024494
dependencies:
45034495
prop-types "^15.5.4"
45044496

4505-
react@^15.6.1:
4506-
version "15.6.2"
4507-
resolved "https://registry.yarnpkg.com/react/-/react-15.6.2.tgz#dba0434ab439cfe82f108f0f511663908179aa72"
4497+
react@^16.4.2:
4498+
version "16.4.2"
4499+
resolved "https://registry.yarnpkg.com/react/-/react-16.4.2.tgz#2cd90154e3a9d9dd8da2991149fdca3c260e129f"
45084500
dependencies:
4509-
create-react-class "^15.6.0"
4510-
fbjs "^0.8.9"
4501+
fbjs "^0.8.16"
45114502
loose-envify "^1.1.0"
4512-
object-assign "^4.1.0"
4513-
prop-types "^15.5.10"
4503+
object-assign "^4.1.1"
4504+
prop-types "^15.6.0"
45144505

45154506
read-only-stream@^2.0.0:
45164507
version "2.0.0"

0 commit comments

Comments
 (0)