React-sortable-hoc: Moving a listitem only works once

[Twisterking]

Hi there!

So I am using your library together with Meteor. Sorting a list works perfectly at first, but it always works only once. I then have to reload the whole page to get it working again.

On second try the following error occurs:

Uncaught TypeError: Cannot read property 'top' of undefined
    at _class.animateNodes (...)
    at _this.handleSortMove (...)

In detail, the error occurs in this line. (if my Chrome debugger is correct the edgeOffset.top part!)

Am I doing something wrong or is this a bug?
I am using your lib based on an array in this.state, so I can't see any interferences with Meteor?!
I hope you can help!

best, P

[Twisterking]

No input on this? Can't see why this is happening ...

[clauderic]

Sorry for the delay, I'm pretty swamped at the moment 😅

Would you be able to provide a code sample I can look at? I've heard of others having similar issues before, in general, it's an issue with not properly updating the state.

[Twisterking]

Hey! Thanks for the reply!
Here is some code:

const SortableItem = SortableElement(({ooitem}) => <Item key={ooitem._id} ooitem={ooitem} />);
const SortableList = SortableContainer(({ooitems, items}) => {
  return (
    <ul className="itemlist">
      {ooitems.map((ooitem, index) => {
        ooitem.item = _.find(items, { _id: ooitem.item._id });
        return <SortableItem key={`item-${index}`} index={index} ooitem={ooitem} />
      })}
    </ul>
  );
});

class ItemListPure extends React.Component {
  constructor(props) {
    super(props);
    this.onSortEnd = this.onSortEnd.bind(this);
    this.state = {
      ooitems: props.ooitems,
      items: props.items
    };
  }

  componentWillReceiveProps(nextProps) {
    this.setState({
      ooitems: nextProps.ooitems,
      items: nextProps.items
    });
  }

  renderItems() {
    var items = this.props.items;
    return this.props.ooitems.map((ooitem) => {
      ooitem.item = _.find(items, { _id: ooitem.item._id });
      return <Item key={ooitem._id} ooitem={ooitem} />
    });
  }

  onSortEnd({oldIndex, newIndex, collection}) {
    this.setState({
      ooitems: arrayMove(this.state.ooitems, oldIndex, newIndex)
    });
  }

  render() {
    if (this.props.loading) {
      return <Loading />
    }
    else if (this.props.ooitems.length > 0) {
      return <SortableList ooitems={this.state.ooitems} items={this.state.items} onSortEnd={this.onSortEnd} />
    } else {
      return (<h3>No openorder</h3>)
    }
  }
}

const ItemListContainer = createContainer((props) => {
  const subscription = Meteor.subscribe('getItems', {
    listId: props.listId,
    mode: props.mode,
    searchQuery: props.searchQuery
  });
  const loading = !subscription.ready();
  return {
    loading,
    ooitems: OpenOrdersBody.find({ list_id: props.listId }, { sort: { row_id: 1 } }).fetch(),
    items: Items.find().fetch()
  }
}, ItemListPure);

As you can see I receive the data from Meteor via props and store them in the state to sort it later.
I already used some console.logging in the onSortEnd function to see if it is working and it seems fine!
When trying to sort a second time the error in my OP occurs once for each element in my list.

[Twisterking]

Any news on this? My app is going into production mode soon and I need this feature to work!
Can I provide you with any more infos or maybe can you provide a hotfix I can implement into a fork?

[ChristopheVandePoel]

Hey @Twisterking. Your very first line:

const SortableItem = SortableElement(({ooitem}) => <Item key={ooitem._id} ooitem={ooitem} />);

Lose the key:

const SortableItem = SortableElement(({ooitem}) => <Item ooitem={ooitem} />);

For some reason it messes up when you add one yourself. (although for me, it did so silently, without the error)

[ms88privat]

@Twisterking For me it helped to put const SortableItem = SortableElement(({ooitem}) => <Item key={ooitem._id} ooitem={ooitem} />); into the render function of the parent List. So this will get re-created every time your list data changes.

Not sure about all the implications this will bring with it, but it worked for me while evaluating this library.

[clauderic]

You should never use HOCs inside of the render method. Not only is there a performance cost to doing this, but the state of that component and all of its children gets lost whenever the render method is called again.

Instead, apply HOCs outside the component definition so that the resulting component is created only once. Then, its identity will be consistent across renders. This is usually what you want, anyway.

See https://facebook.github.io/react/docs/higher-order-components.html#dont-use-hocs-inside-the-render-method for more info.

[ms88privat]

@clauderic yes, you are right. In this case I misused it explicitly to make it work. And I carefully controlled the parent elements render, so it will only be re-rendered if it has to. But anyway, was just a quick evaluation and not digging into the real issue here :)

[clauderic]

No worries, just wanted to set the record straight for any newcomers landing on this issue 😄