react-sortable-hoc: Uncaught TypeError: Cannot read property 'top' of undefined(…)

I still haven’t reproduced it in JSFiddle - but I think I’ve found the source of the bug and the solution, in my project at least.

The issue was that components wrapped by SortableElement were being re-rendered but the wrapping SortableElement component was not. This meant that the componentWillUnmount event on SortableElement was not firing so the refs in the manager class were not being updated, resulting in the refs pointing at DOM nodes that had been removed by the re-render.

The fix was to set the key prop of the SortableElement component to a string dependent on the information displayed within the wrapped component. This means that if the data changes such that the wrapped component needs to re-render, the wrapping SortableElement will have it’s key changed causing it to re-render as well which will trigger the lifecycle events that will keep the manager class refs in sync.

So we’ve got a wrapped SortableElement:

const Card = SortableElement(MyElement);

And an array containing the data for each card which we want to map over to generate the list. React will complain if the sibling items in the list do not have a unique key set - the solution I usually use for this is to use the array index to build the key:

{cards.map((card, index) => {
  return (
    <Card
      key={`card-${index}`}
      data={card}
    />
  );
})}

However, that doesn’t work in this case. Sometimes the array index will stay the same, but the component needs to be re-rendered for react-sortable-hoc to work. If the data prop changes the wrapped component is re-rendered, but the wrapping component is not!

That’s what causes the weird behavior, all of the components whose index changed work fine - the ones where the index stayed the same no longer work. You can check out this behavior in React-Devtools.

The solution I’ve been using is to make it so that the key is dependent on the data in card. This will force react to re-render the wrapping component whenever the data changes, rather than when it’s index changes:

{cards.map((card) => {
  return (
    <Card
      key={`card-${card.title}-${card.content}`}
      data={card}
     />
   );
})}

I hope that helps 😃

That’s not something I would recommend @erisnuts. This will needlessly cause many nodes to unmount and re-mount. You should always use predictable, consistent keys where possible.

https://facebook.github.io/react/docs/reconciliation.html#tradeoffs

I have a totally opposite situation, when I use index as the key they all work fine, but after I changing to use item-related content as key they stuck in the list after initial move. No idea why is that.

Edit: Problem solved. Must use something that changes during the sorting process as the key. And if you use the index as the key, that’s another problem, every wrapped element is new when sorting, so their states won’t persist.

hello! I also am having the same issue and the above suggestions did not help, still getting the original error

It seems like things go wrong when I delete an item, then the UI and the data are out of sync, so it sounds like @TimHollies suggestion above about the wrapped components being re-rendered but not the wrapping component, however I wasn’t able to get the suggestion to work

Any help or guidance sincerely appreciated!

const SortableItem = SortableElement(({ item, itemIndex, ...rest }) => (
  <ListItem
    key={item.itemId}
    useDragHandle
    item={item}
    index={itemIndex}
    onClickCheckBox={() => rest.onClickCheckBox(itemIndex)}
    onItemTextUpdate={value =>
      rest.onItemTextUpdate(value, item.itemId, itemIndex)
    }
    onDelete={() => rest.onDelete(item.itemId)}
  >
    {item.text}
  </ListItem>
))

const SortableList = SortableContainer(({ items, ...rest }) => (
  <StyledItemList>
    {items.map((item, index) => (
      <SortableItem
        key={`item-${item.itemId}-${item.text}`}
        itemIndex={index}
        index={index}
        item={item}
        {...rest}
      />
    ))}
  </StyledItemList>
))

class MobileListItems extends Component {
  state = {
    items: this.props.listItems,
  }

  onSortEnd = ({ oldIndex, newIndex }) => {
    this.setState(({ items }) => ({
      items: arrayMove(items, oldIndex, newIndex),
    }))
  }

  render() {
    const { onClickCheckBox, onDelete, onItemTextUpdate } = this.props

    return (
      <SortableList
        onClickCheckBox={index => onClickCheckBox(index)}
        onDelete={onDelete}
        onItemTextUpdate={onItemTextUpdate}
        items={this.state.items}
        onSortEnd={this.onSortEnd}
      />
    )
  }
}

Great, thanks @TimHollies! I had the same issue with SortableElement state being swapped between the dragged elements. Changing the key to an element specific value instead of using index did the trick.