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.

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

Report a warning when using props data as items. #82

Closed
insekkei opened this issue Nov 7, 2016 · 8 comments
Closed

Report a warning when using props data as items. #82

insekkei opened this issue Nov 7, 2016 · 8 comments

Comments

@insekkei
Copy link

insekkei commented Nov 7, 2016

Code:

Component.js:

import {SortableContainer, SortableElement} from 'react-sortable-hoc';

let introModuleList = this.props.introModuleList;

const SortableItem = SortableElement(({value}) => {
 return <div className="module">
     <Button onClick={() => this.deleteModule(value.moduleOrder)}>删除</Button>
     <div>
         {this.getCurrentModule(value)}
     </div>
 </div>;
});

const SortableList = SortableContainer(({items}) => {
 return (
     <div>
         {items.map((item, index) =>
             <SortableItem key={`item-${index}`} index={index} value={item} />
         )}
     </div>
 );
});

return <SortableList items={introModuleList} onSortEnd={this.onSortEnd} />;

onSortEnd = ({oldIndex, newIndex}) => {
   this.props.changeModuleOrder(oldIndex, newIndex);
};

Reducer:

import {arrayMove} from 'react-sortable-hoc';

arrayMove(initialState.introModuleList, oldIndex, newIndex)

Result:

warning.js:45 Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the SortableList component.

@insekkei
Copy link
Author

insekkei commented Nov 7, 2016

insekkei@5f3fb69

After removethis.setState({...}) the warning disappeared and everything goes well...

@clauderic
Copy link
Owner

@insekkei would you be able to provide a more complete code sample or share a gist / jsfiddle? It's very hard for me to help you out based only on what you've provided so far, I'd need to see more.

@ekrokowski
Copy link

ekrokowski commented Jan 6, 2017

@clauderic The problem here is that when using redux the component may get re-rendered within the onSortEnd Callback. This will unmount the SortableList Component.

Since the onSortEnd Callback is fired before setState is called, the component is already unmounted and the Error Warning: setState(...): Can only update a mounted or mounting component. This usually means you called setState() on an unmounted component. This is a no-op. Please check the code for the SortableList component. will come up.

Is there any reason the provided onSortEnd Callback from props is fired before the setState is used? See here: https://github.com/clauderic/react-sortable-hoc/blob/master/src/SortableContainer/index.js#L307

I think we should simply put that block at the end of the handleSortEnd-Function, just like here (my fork) -> https://github.com/ekrokowski/react-sortable-hoc/blob/51f2c4d3ac763639e66fbd6659cdd8c5fd568cbe/src/SortableContainer/index.js#L302

@sag1v
Copy link

sag1v commented Jan 20, 2017

i'm having the same issue :/ anyone found the problem or the fix to this?

@ekrokowski
Copy link

@sag1v Read my comment..

In the handleSortEnd function -> move this:

if (typeof onSortEnd === 'function') {
	onSortEnd({
		oldIndex: this.index,
		newIndex: this.newIndex,
		collection
	}, e);
}

after the setState (or just to the end of the function)

this.setState({
	sorting: false,
	sortingIndex: null
});

Just like in my fork: https://github.com/ekrokowski/react-sortable-hoc/blob/51f2c4d3ac763639e66fbd6659cdd8c5fd568cbe/src/SortableContainer/index.js#L302

@sag1v
Copy link

sag1v commented Jan 20, 2017

Hmm thanks man, but changing it feels awkward. Why isnt the aithor approve this change? Does it have any side effects?

@microdou
Copy link

microdou commented Jan 24, 2017

@ekrokowski
I had the same issue. Thanks for your fix! I tried your fork and it worked for me.
I think you can submit a pull request to the author.

@clauderic
Copy link
Owner

Hey @ekrokowski, thanks for pointing that out. Just shipped 0.4.12 with that change

DimitarNestorov pushed a commit to codemotionapps/react-sortable-hoc that referenced this issue Feb 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants