Skip to content

Commit

Permalink
chore: Migrating to async safe lifecycle functions (#320)
Browse files Browse the repository at this point in the history 
* added deps

* updating eslint

* migrating to gDSFP

* make sure treeData is from instanceProps

* fix test
  • Loading branch information
@wuweiweiwu
wuweiweiwu committed Jun 1, 2018
1 parent 2706488 commit c844c04
Show file tree
Hide file tree
Showing 4 changed files with 117 additions and 74 deletions.
4 changes: 3 additions & 1 deletion .eslintrc.json → .eslintrc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
},
"rules": {
"react/jsx-filename-extension": 0,
"react/prefer-stateless-function": 0
"react/prefer-stateless-function": 0,
"react/no-did-mount-set-state": 0,
"react/sort-comp": 0
}
}
5 changes: 3 additions & 2 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"react-dnd": "2.6.0",
"react-dnd-html5-backend": "2.6.0",
"react-dnd-scrollzone": "^4.0.0",
"react-lifecycles-compat": "^3.0.4",
"react-virtualized": "^9.19.1"
},
"peerDependencies": {
Expand Down Expand Up @@ -104,11 +105,11 @@
"json-loader": "^0.5.7",
"postcss-loader": "^2.1.5",
"prettier": "^1.12.1",
"react": "^16.3.2",
"react": "^16.2.0",
"react-addons-shallow-compare": "^15.6.2",
"react-dnd-test-backend": "2.6.0",
"react-dnd-touch-backend": "0.4.0",
"react-dom": "^16.3.2",
"react-dom": "^16.2.0",
"react-hot-loader": "^4.2.0",
"react-sortable-tree-theme-file-explorer": "^1.1.2",
"react-test-renderer": "^16.3.2",
Expand Down
170 changes: 105 additions & 65 deletions src/react-sortable-tree.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import withScrolling, {
createVerticalStrength,
createHorizontalStrength,
} from 'react-dnd-scrollzone';
import { polyfill } from 'react-lifecycles-compat';
import 'react-virtualized/styles.css';
import TreeNode from './tree-node';
import NodeRendererDefault from './node-renderer-default';
Expand Down Expand Up @@ -108,6 +109,14 @@ class ReactSortableTree extends Component {
searchMatches: [],
searchFocusTreeIndex: null,
dragging: false,

// props that need to be used in gDSFP or static functions will be stored here
instanceProps: {
treeData: [],
ignoreOneTreeUpdate: false,
searchQuery: null,
searchFocusOffset: null,
},
};

this.toggleChildrenVisibility = this.toggleChildrenVisibility.bind(this);
Expand All @@ -120,8 +129,15 @@ class ReactSortableTree extends Component {
}

componentDidMount() {
this.loadLazyChildren();
this.search(this.props);
ReactSortableTree.loadLazyChildren(this.props, this.state);
const stateUpdate = ReactSortableTree.search(
this.props,
this.state,
true,
true,
false
);
this.setState(stateUpdate);

// Hook into react-dnd state changes to detect when the drag ends
// TODO: This is very brittle, so it needs to be replaced if react-dnd
Expand All @@ -131,34 +147,49 @@ class ReactSortableTree extends Component {
.subscribeToStateChange(this.handleDndMonitorChange);
}

componentWillReceiveProps(nextProps) {
if (this.props.treeData !== nextProps.treeData) {
// Ignore updates caused by search, in order to avoid infinite looping
if (this.ignoreOneTreeUpdate) {
this.ignoreOneTreeUpdate = false;
} else {
// Reset the focused index if the tree has changed
this.setState({ searchFocusTreeIndex: null });
static getDerivedStateFromProps(nextProps, prevState) {
const { instanceProps } = prevState;
const newState = {};

// Load any children defined by a function
this.loadLazyChildren(nextProps);
// make sure we have the most recent version of treeData
instanceProps.treeData = nextProps.treeData;

This comment has been minimized.

Sign in to view

Copy link
@digitalpbk

digitalpbk Jul 16, 2018

@wuweiweiwu The above line causes line 157 to be always false, breaks lazy loading.

This comment has been minimized.

Sign in to view

Copy link
@wuweiweiwu

wuweiweiwu Jul 17, 2018

Author Member

Nice catch. Ill have to patch that

this.search(nextProps, false, false);
if (!isEqual(instanceProps.treeData, nextProps.treeData)) {
if (instanceProps.ignoreOneTreeUpdate) {
instanceProps.ignoreOneTreeUpdate = false;
} else {
newState.searchFocusTreeIndex = null;
ReactSortableTree.loadLazyChildren(nextProps, prevState);
Object.assign(
newState,
ReactSortableTree.search(nextProps, prevState, false, false, false)
);
}

// Reset the drag state
this.setState({
draggingTreeData: null,
draggedNode: null,
draggedMinimumTreeIndex: null,
draggedDepth: null,
dragging: false,
});
} else if (!isEqual(this.props.searchQuery, nextProps.searchQuery)) {
this.search(nextProps);
} else if (this.props.searchFocusOffset !== nextProps.searchFocusOffset) {
this.search(nextProps, true, true, true);
newState.draggingTreeData = null;
newState.draggedNode = null;
newState.draggedMinimumTreeIndex = null;
newState.draggedDepth = null;
newState.dragging = false;
} else if (!isEqual(instanceProps.searchQuery, nextProps.searchQuery)) {
Object.assign(
newState,
ReactSortableTree.search(nextProps, prevState, true, true, false)
);
} else if (
instanceProps.searchFocusOffset !== nextProps.searchFocusOffset
) {
Object.assign(
newState,
ReactSortableTree.search(nextProps, prevState, true, true, true)
);
}

instanceProps.searchQuery = nextProps.searchQuery;
instanceProps.searchFocusOffset = nextProps.searchFocusOffset;
newState.instanceProps = instanceProps;

return newState;
}

// listen to dragging
Expand Down Expand Up @@ -197,8 +228,10 @@ class ReactSortableTree extends Component {
}

toggleChildrenVisibility({ node: targetNode, path }) {
const { instanceProps } = this.state;

const treeData = changeNodeAtPath({
treeData: this.props.treeData,
treeData: instanceProps.treeData,
path,
newNode: ({ node }) => ({ ...node, expanded: !node.expanded }),
getNodeKey: this.props.getNodeKey,
Expand Down Expand Up @@ -250,46 +283,40 @@ class ReactSortableTree extends Component {
});
}

search(
props = this.props,
seekIndex = true,
expand = true,
singleSearch = false
) {
// returns the new state after search
static search(props, state, seekIndex, expand, singleSearch) {
const {
treeData,
onChange,
getNodeKey,
searchFinishCallback,
searchQuery,
searchMethod,
searchFocusOffset,
onlyExpandSearchedNodes,
} = props;

// Skip search if no conditions are specified
if (
(searchQuery === null ||
typeof searchQuery === 'undefined' ||
String(searchQuery) === '') &&
!searchMethod
) {
this.setState({
searchMatches: [],
});
const { instanceProps } = state;

// Skip search if no conditions are specified
if (!searchQuery && !searchMethod) {
if (searchFinishCallback) {
searchFinishCallback([]);
}

return;
return { searchMatches: [] };
}

const newState = {};

// if onlyExpandSearchedNodes collapse the tree and search
const { treeData: expandedTreeData, matches: searchMatches } = find({
getNodeKey: this.props.getNodeKey,
getNodeKey,
treeData: onlyExpandSearchedNodes
? toggleExpandedForAll({ treeData, expanded: false })
: treeData,
? toggleExpandedForAll({
treeData: instanceProps.treeData,
expanded: false,
})
: instanceProps.treeData,
searchQuery,
searchMethod: searchMethod || defaultSearchMethod,
searchFocusOffset,
Expand All @@ -299,7 +326,7 @@ class ReactSortableTree extends Component {

// Update the tree with data leaving all paths leading to matching nodes open
if (expand) {
this.ignoreOneTreeUpdate = true; // Prevents infinite loop
newState.ignoreOneTreeUpdate = true; // Prevents infinite loop
onChange(expandedTreeData);
}

Expand All @@ -316,20 +343,20 @@ class ReactSortableTree extends Component {
searchFocusTreeIndex = searchMatches[searchFocusOffset].treeIndex;
}

this.setState({
searchMatches,
searchFocusTreeIndex,
});
newState.searchMatches = searchMatches;
newState.searchFocusTreeIndex = searchFocusTreeIndex;

return newState;
}

startDrag({ path }) {
this.setState(() => {
this.setState((prevState) => {
const {
treeData: draggingTreeData,
node: draggedNode,
treeIndex: draggedMinimumTreeIndex,
} = removeNode({
treeData: this.props.treeData,
treeData: prevState.instanceProps.treeData,
path,
getNodeKey: this.props.getNodeKey,
});
Expand All @@ -349,6 +376,8 @@ class ReactSortableTree extends Component {
depth: draggedDepth,
minimumTreeIndex: draggedMinimumTreeIndex,
}) {
const { instanceProps } = this.state;

// Ignore this hover if it is at the same position as the last hover
if (
this.state.draggedDepth === draggedDepth &&
Expand All @@ -359,7 +388,8 @@ class ReactSortableTree extends Component {

// Fall back to the tree data if something is being dragged in from
// an external element
const draggingTreeData = this.state.draggingTreeData || this.props.treeData;
const draggingTreeData =
this.state.draggingTreeData || instanceProps.treeData;

const addedResult = memoizedInsertNode({
treeData: draggingTreeData,
Expand Down Expand Up @@ -391,6 +421,8 @@ class ReactSortableTree extends Component {
}

endDrag(dropResult) {
const { instanceProps } = this.state;

const resetTree = () =>
this.setState({
draggingTreeData: null,
Expand All @@ -415,13 +447,13 @@ class ReactSortableTree extends Component {
});
}

let treeData = this.state.draggingTreeData || this.props.treeData;
let treeData = this.state.draggingTreeData || instanceProps.treeData;

// If copying is enabled, a drop outside leaves behind a copy in the
// source tree
if (shouldCopy) {
treeData = changeNodeAtPath({
treeData: this.props.treeData, // use treeData unaltered by the drag operation
treeData: instanceProps.treeData, // use treeData unaltered by the drag operation
path,
newNode: ({ node: copyNode }) => ({ ...copyNode }), // create a shallow copy of the node
getNodeKey: this.props.getNodeKey,
Expand All @@ -448,10 +480,13 @@ class ReactSortableTree extends Component {
}

// Load any children in the tree that are given by a function
loadLazyChildren(props = this.props) {
// calls the onChange callback on the new treeData
static loadLazyChildren(props, state) {
const { instanceProps } = state;

walk({
treeData: props.treeData,
getNodeKey: this.props.getNodeKey,
treeData: instanceProps.treeData,
getNodeKey: props.getNodeKey,
callback: ({ node, path, lowerSiblingCounts, treeIndex }) => {
// If the node has children defined by a function, and is either expanded
// or set to load even before expansion, run the function.
Expand All @@ -469,9 +504,9 @@ class ReactSortableTree extends Component {

// Provide a helper to append the new data when it is received
done: childrenArray =>
this.props.onChange(
props.onChange(
changeNodeAtPath({
treeData: this.props.treeData,
treeData: instanceProps.treeData,
path,
newNode: ({ node: oldNode }) =>
// Only replace the old node if it's the one we set off to find children
Expand All @@ -482,7 +517,7 @@ class ReactSortableTree extends Component {
children: childrenArray,
}
: oldNode,
getNodeKey: this.props.getNodeKey,
getNodeKey: props.getNodeKey,
})
),
});
Expand All @@ -492,9 +527,11 @@ class ReactSortableTree extends Component {
}

renderRow(
{ node, parentNode, path, lowerSiblingCounts, treeIndex },
row,
{ listIndex, style, getPrevRow, matchKeys, swapFrom, swapDepth, swapLength }
) {
const { node, parentNode, path, lowerSiblingCounts, treeIndex } = row;

const {
canDrag,
generateNodeProps,
Expand Down Expand Up @@ -572,9 +609,10 @@ class ReactSortableTree extends Component {
draggedNode,
draggedDepth,
draggedMinimumTreeIndex,
instanceProps,
} = this.state;

const treeData = this.state.draggingTreeData || this.props.treeData;
const treeData = this.state.draggingTreeData || instanceProps.treeData;

let rows;
let swapFrom = null;
Expand Down Expand Up @@ -870,6 +908,8 @@ ReactSortableTree.contextTypes = {
dragDropManager: PropTypes.shape({}),
};

polyfill(ReactSortableTree);

// Export the tree component without the react-dnd DragDropContext,
// for when component is used with other components using react-dnd.
// see: https://github.com/gaearon/react-dnd/issues/186
Expand Down
12 changes: 6 additions & 6 deletions yarn.lock
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8593,9 +8593,9 @@ react-docgen@^3.0.0-beta11:
node-dir "^0.1.10"
recast "^0.12.6"

react-dom@^16.3.2:
version "16.3.2"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.3.2.tgz#cb90f107e09536d683d84ed5d4888e9640e0e4df"
react-dom@^16.2.0:
version "16.4.0"
resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.4.0.tgz#099f067dd5827ce36a29eaf9a6cdc7cbf6216b1e"
dependencies:
fbjs "^0.8.16"
loose-envify "^1.1.0"
Expand Down Expand Up @@ -8747,9 +8747,9 @@ react-virtualized@^9.19.1:
prop-types "^15.6.0"
react-lifecycles-compat "^3.0.4"

react@^16.3.2:
version "16.3.2"
resolved "https://registry.yarnpkg.com/react/-/react-16.3.2.tgz#fdc8420398533a1e58872f59091b272ce2f91ea9"
react@^16.2.0:
version "16.4.0"
resolved "https://registry.yarnpkg.com/react/-/react-16.4.0.tgz#402c2db83335336fba1962c08b98c6272617d585"
dependencies:
fbjs "^0.8.16"
loose-envify "^1.1.0"
Expand Down

0 comments on commit c844c04

Please sign in to comment.