Major change proposal: Let user decide what goes into state

[gaearon]

Currently we have functions like getDragState(type), getDropState(type), getDragLayerState(). They can be used from render but they're limiting in several ways:

• You can't use them in componentDidUpdate or similar lifecycle hooks, you can't compare with previous values and thus react to changes.
• Even with PureRenderMixin, you can't opt out of reconciling when the state of mixin changes. For example, you can't make a drag layer that would only update in 10 pixel steps. If you were implementing it yourself, you could have put Math.round(currentOffset.x) into state.x and use PureRenderMixin to avoid extra changes, but getDragLayerState() is opaque and uses underlying component's state as it wishes.

I wonder if we should change the contract so user defines methods that tell us what to keep in state. For example, drag panel that rounds values:

var DragStatePanel = React.createClass({
  mixins: [DragLayerMixin, PureRenderMixin],

  // NEW: let user pick the data
  collectDragLayerState(context) {
    // it's the same familiar context that is passed to configureDragDrop! no magic!
    var isDragging = context.isDragging(),
        clientOffset = context.getCurrentOffsetFromClient();

    return {
      isDragging: isDragging,
      roundedX: isDragging ? Math.round(clientOffset.x / 10) * 10 : null,
      roundedX: isDragging ? Math.round(clientOffset.y / 10) * 10 : null,
    }
  },

  render() {
    // NEW: read directly from your own state
    var { roundedX, roundedY, isDragging } = this.state;
    if (!isDragging) {
      return null;
    }

    return (
      <div>
        <p>Dragging at coordinates: {roundedX}, {roundedY}.</p>
      </div>
    );
  }
});

Similarly, for drop targets (drag sources are the same):

  // NEW: decide what to pick
  collectDropTargetState(context) {
    return {
      isOver: context.isOver(ItemTypes.ITEM),
      isDragging: context.isDragging(ItemTypes.ITEM)
    };
  },

  componentDidUpdate(prevProps, prevState) {
    if (prevState.isOver && !this.state.isOver) {
      // NEW! do we even need enter() and leave() now?
    }
  },

  render() {
    // NEW: take values from your own state
    var { isOver, isDragging } = this.state,
        backgroundColor = '#222';

    if (isOver) {
      backgroundColor = 'darkgreen';
    } else if (isDragging) {
      backgroundColor = 'darkkhaki';
    }

    return (
      <div {...this.dropTargetFor(ItemTypes.ITEM)}
           style={{ backgroundColor }}>
        {isOver ?
          'Release to drop' :
          'Drag item here'
        }
      </div>
    );
  }

If we can do this we might even not need enter and leave that much. I'd like to eventually move away from them in favor of React's componentDidUpdate. This would also allow us not to add enterCurrent and friends because consumer can just use componentDidUpdate for that.

We'd still need over though. Not sure what can be done there.

[nelix]

I really like the idea of leaving it to the consumer to construct their state. It avoids some of the issues with mixins that set state (clashing names and discoverability mostly). In general it seems more correct and flexible also.

collectDropTargetState does this mean its only for targets and not sources? In general though I love this idea.

[nelix]

Could over be handled with something like this for source? Or am I missing the goal of over?

[gaearon]

does this mean its only for targets and not sources? In general though I love this idea.

I meant there to be separate methods like collectDragSourceState, collectDropTargetState, collectDragLayerState.

Or am I missing the goal of over?

Usually in over you want just the side effect. It may not really be expressible with a state change you “react” to.

[nelix]

This sounds pretty good. 👍

[gaearon]

We're ready to go ahead with this.
dnd-core already has everything we need except for over hook which we'll add soon.

Things to be considered:

• How do we change drag source / target API?
  • We want to remove enter and leave
  • We want to rename over to dragOver
  • Do we really want drag source / drop target API to differ from dnd-core? In some ways, it should differ because we want to be able to specify things like dragPreview that are specific to React DnD. Instead of passing context and handle, we want hooks to accept context and component, etc. So they're clearly not the same thing. How are they constructed? dnd-core prefers classes (or rather makes them easy to work with), React DnD currently uses plain objects. We need to decide that. We'll probably stick with wrapping sources/targets provided to React DnD into some dnd-core style wrappers before passing them to dnd-core. Similarly, context passed by React DnD to hooks will likely wrap dnd-core's context.
  • Exotic things like dragPreview, effectsAllowed, etc. I need them to work (I actually use them). Need to figure out the right API for them.
  • How many collect* methods do we need? Is it possible to pass them to mixin in its declaration instead of defining them on component? Can we do the same for registration?
• Support native drag sources
• How do we solve coordinate tracking? See How do we implement dragOver and coordinate tracking? dnd-core#6 for thoughts on this. We might come back to this when everything else is ready.

Overall the right course is to start from scratch and make examples work, from the simplest one to more complex ones.

[gaearon]

Let's do the work in this branch: https://github.com/gaearon/react-dnd/tree/dnd-core

[gaearon]

Superseded by #111, let's track progress there.