Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

No setState(null) #1129

Merged
merged 2 commits into from
Jun 1, 2018
Merged

No setState(null) #1129

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cbdfce4
react 15 throws warning if setState `null`
wuweiweiwu Jun 1, 2018
23f3d66
removing extra return null
wuweiweiwu Jun 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
54 changes: 24 additions & 30 deletions source/Grid/Grid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,7 @@ class Grid extends React.PureComponent<Props, State> {
: SCROLL_DIRECTION_BACKWARD
: this.state.scrollDirectionVertical;

const newState: Object = {
const newState: $Shape<State> = {
isScrolling: true,
scrollDirectionHorizontal,
scrollDirectionVertical,
Expand Down Expand Up @@ -597,29 +597,26 @@ class Grid extends React.PureComponent<Props, State> {
// In that event we need to remeasure.
if (!instanceProps.scrollbarSizeMeasured) {
this.setState(prevState => {
prevState.instanceProps.scrollbarSize = getScrollbarSize();
prevState.instanceProps.scrollbarSizeMeasured = true;
prevState.needToResetStyleCache = false;
return prevState;
const stateUpdate = {...prevState, needToResetStyleCache: false};
stateUpdate.instanceProps.scrollbarSize = getScrollbarSize();
stateUpdate.instanceProps.scrollbarSizeMeasured = true;
return stateUpdate;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it ok here to mutate state?

Copy link
Collaborator Author

@wuweiweiwu wuweiweiwu Jun 1, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. previously it was this before the gDSFP migration

And since I need to set stuff on instanceProps I use prevState as a base

});
}

if (
(typeof scrollLeft === 'number' && scrollLeft >= 0) ||
(typeof scrollTop === 'number' && scrollTop >= 0)
) {
this.setState(prevState => {
const stateUpdate = Grid._getScrollToPositionStateUpdate({
prevState,
scrollLeft,
scrollTop,
});
if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
return stateUpdate;
}
return null;
const stateUpdate = Grid._getScrollToPositionStateUpdate({
prevState: this.state,
scrollLeft,
scrollTop,
});
if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
this.setState(stateUpdate);
}
}

// setting the ref's scrollLeft and scrollTop.
Expand Down Expand Up @@ -1433,19 +1430,16 @@ class Grid extends React.PureComponent<Props, State> {
* Useful for animating position changes.
*/
scrollToPosition({scrollLeft, scrollTop}: ScrollPosition) {
this.setState(prevState => {
const stateUpdate = Grid._getScrollToPositionStateUpdate({
prevState,
scrollLeft,
scrollTop,
});

if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
return stateUpdate;
}
return null;
const stateUpdate = Grid._getScrollToPositionStateUpdate({
prevState: this.state,
scrollLeft,
scrollTop,
});

if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
this.setState(stateUpdate);
}
}

static _wrapSizeGetter(value: CellSize): CellSizeGetter {
Expand Down Expand Up @@ -1527,8 +1521,8 @@ class Grid extends React.PureComponent<Props, State> {
);
if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
this.setState(stateUpdate);
}
this.setState(stateUpdate);
}

static _getCalculatedScrollTop(nextProps: Props, prevState: State) {
Expand Down Expand Up @@ -1623,8 +1617,8 @@ class Grid extends React.PureComponent<Props, State> {
);
if (stateUpdate) {
stateUpdate.needToResetStyleCache = false;
this.setState(stateUpdate);
}
this.setState(stateUpdate);
}

_onScroll = (event: Event) => {
Expand Down