Warning: Cannot update a component from inside the function body of a different component.

[digilist]

Describe the bug
The warning Warning: Cannot update a component from inside the function body of a different component. appears with the latest React version 16.13 when using the example provided in the readme file.

To Reproduce
Steps to reproduce the behavior: Create a new component using the example code with React v16.13.0

Expected behavior
No warning :)

Information
This happens because the setList method is passed to the child component which results now in a warning.

More information in the react release notes: https://reactjs.org/blog/2020/02/26/react-v16.13.0.html

Versions
react-sortable = ^2.0.11
react = ^16.13.0

[swba]

This is because ReactSortable is compiled into a function that explicitly calls

props.setList(newList, _this.sortable, store);

If this was source code, wrapping this call with useEffect would fix the issue:

useEffect(() => { props.setList(newList, _this.sortable, store); }, []);

Bad news is that according to facebook/react#18178 (comment) this error may break the whole ReactSortable component in the nearest future...

[waynevanson]

After some digging, this comment describes the issue well: facebook/react#18178 (comment)

This happens because the setList method is passed to the child component which results now in a warning.

@digilist I don't see anywhere in the code where setList is passed to a child component. Would you be able to elaborate on this?

It looks like the error may be coming from this LOC, within the constructor:

react-sortablejs/src/react-sortable.tsx

Line 56 in 5ec3548

props.setList(newList, this.sortable, store);

I hypothesize that we'll need to remove this from the constructor (because this may be during rendering), wait for the component to render, then call it somewhere else (componentDidUpdate, but only after first render).

If this was source code, wrapping this call with useEffect would fix the issue:

This component does not use hooks thus far. I'm actually trying to port this to hooks because it'll be easier to deal fix issues with.

[digilist]

@waynevanson sorry, I think I didn't express it well. I think it's coming from the callback that is passed via the setList parameter as it is used in the example here: https://github.com/SortableJS/react-sortablejs#function-component

[stevecastaneda]

Also running into this warning on ^16.13.0.

[hgeldenhuys]

I also got this is, but it was the list property, not setList. If you use useState or useSelector as your list you get this warning. If I do a "cheap" clone with JSON.parse(JSON.stringify(listFromStateOrSelector))
the warning disappears.

[stevecastaneda]

@hgeldenhuys I was able to confirm that fixed it here, but oddly enough, when I reverted back to using the state directly, the error was gone. 🤔

[waynevanson]

I'm not quire understanding how to replicate the issue. Would someone be able to put this into a codesandbox? I want to make sure I'm not just changing code for the hell of it.

[digilist]

Probably the warning is gone with the latest react versions. In my application I do not see the warning anymore 🤔

[stevecastaneda]

@digilist I agree, now on 16.13.1 and not seeing the error.

https://github.com/facebook/react/releases/tag/v16.13.1

Likely due to this?

Revert warning for cross-component updates that happen inside class render lifecycles (componentWillReceiveProps, shouldComponentUpdate, and so on). (@gaearon in #18330)

[gaearon]

Yeah.

It still pointed out a legit problem but we’ve silenced it for classes because it’s “too late” to fix there. People tend to treat constructors as places to do side effects in some legacy code.

[waynevanson]

I'll still address the issue. It's becoming obvious that setlist shouldn't be called within the constructor

[webdesign2be]

@waynevanson if I can help you to refactor to hooks and functional components, just let me know. :)

https://codesandbox.io/s/affectionate-blackburn-7et9h?file=/src/App.js

[eppisapia-legacy]

I also got this is, but it was the list property, not setList. If you use useState or useSelector as your list you get this warning. If I do a "cheap" clone with JSON.parse(JSON.stringify(listFromStateOrSelector))
the warning disappears.

I tried working with a copy of the list as you say but it still doesn't update the component inner state after change index

[eppisapia-legacy]

I'm not quire understanding how to replicate the issue. Would someone be able to put this into a codesandbox? I want to make sure I'm not just changing code for the hell of it.

There is a sandbox with issue
https://codesandbox.io/s/intelligent-paper-iodho?file=/src/App.js

[siwonia]

Maybe not the best solution, but adding a setTimeout solves it for me for now.

<ReactSortable
  list={items}
  setList={(nextItems) => setTimeout(() => setItems(nextItems))}
>
  {items.map((item) => (
    <Item key={item.id} {...item} />
  ))}
</ReactSortable>

https://codesandbox.io/s/sweet-varahamihira-zdb19

[waynevanson]

will be fixed in #175 by adding hooks

[nakedgremlin]

@waynevanson Do you know when that update #175 will be set to release? I'm encountering this exact same issue and we are working through a final QA pass before we noticed this issue surface.

I would love to stay with this library since it's been the most straightforward to integrate into our builds.

[velosipedist]

Maybe this would help someone. I just left setList empty and used onEnd handler instead, and called a state update from it. Inside the reducer, I handle the elements s

 <ReactSortable
      list={optionsVal}
      onEnd={(evt, sortable, store) => {
        if (evt.oldIndex === undefined || evt.newIndex === undefined) {
          return;
         }
        dispatch({ type: "OPTION_MOVED", from: evt.oldIndex, to: evt.newIndex });
      }}
      setList={() => {
         // just because it is required
       }}              
     >...list...</ReactSortable>

Catching that in the reducer, I handle swapping manually:

moveItem(options, fromIndex, toIndex);

export function moveItem<T>(items: T[], from: number, to: number): T[] {
  const moveResult = [...items];
  const moved = items[from];
  moveResult.splice(from, 1);
  moveResult.splice(to, 0, moved);
  return moveResult;
}