Error: "No suitable position found to insert" with getNodeKey specified and canDrop returning false

[rrhvella]

I am receiving the error "No suitable position found to insert" when performing a very specific drag under a very specific conditions.

As far as I can tell, the key conditions are as follows:

• getNodeKey must be specified.
• There must be 3 top level nodes.
• The second node must have three levels of children, all of which must be expanded.
• The third node must have at least one child, and must not be expanded.
• canDrop must sometimes return false. I had a very complex condition when I first encountered this problem, but in theory the tree should never fire an error even if canDrop always returns false.

You can find a reproduction of these conditions in this sandbox

Just drag Y under X and you will get the error.

Please note that this problem wasn't present in version 1.5.0 but is present in both 1.7.0 and 2.0.1.

The error is the same as this other issue but the conditions for causing it are completely different so it is likely to be a different problem -- also this issue does not require a quick succession of actions in order to replicate it. This is the reason why I have opened a different issue.

[rrhvella]

I stepped through the code and realized that the only reason the error is not thrown when the third node is expanded is that the addedResult incorrectly states that I am trying to drop the subtree into the third node, when in fact I am dropping it in the first.

[fritz-c]

Thank you for bringing this to my attention and creating an example to reproduce it. This will take high priority when I can secure some time to work on open source again.

[votiv]

Hi! Is there a workaround for this issue?

[datoslabs]

Hi,

I have encountered the same error and found trapping the Exception as @rrhvella suggested seems to be a viable temporary patch. Given many people have reported the same issue and no visible progress has been made since 2018, instead of waiting longer to identify the root cause and permanent fix, I wonder if simply adding the try/catch clause and make a patch release would be worthwhile. I have tested it in my environment, trapping the Exception does not appear to cause any undesirable side-effects; on the other hand, the unhandled exception causes major stability issue for me (and I am sure for many others) which may prohibit the use of this project in production.

Also, I have found that in my situation, the error occurred while I was still dragging. The call to memoizedInsertNode is merely an "optimistic" insert before calling customCanDrop function to determine if the node can be dropped. I have seen the same parameters provided to calling the memoizedInsertNode function succeed during the same drag session without error suggesting a problem with an inconsistent internal state that may have occurred a moment in time. By returning false in the catch clause when error occurred is rather harmless as it will prevent the user from inserting the node (as it should) and continue dragging until memoizedInsertNode call returns successfully (which it eventually would).

[datoslabs]

Sharing an update after extensive debugging. I have a setup that can easily trigger this bug; in my case, the dropTargetProps.listIndex contained an incorrect value as such the memoizedInsertNode call resulted in 0 node insert and triggered the exception. My setup is as follows:

Level 0 - node index 0
    Level 1 - node index 1
        Level 2 - node index 2
        Level 2 - node index 3
    Level 1 - node index 4
        Level 2 - node index 5
        Level 2 - node index 6
Level 0 - node index 7 (collapsed)
    Level 1 - node index 8 (hidden)

When dragging Level 1 - node index 4 up slightly (and slowly) just enough to trigger the drag but keep it hovering at the original location below Level 2 - node index 3 and above Level 0 - node index 7 (collapsed), it generated minimumTreeIndex === dropTargetProps.listIndex === 6 while Level 0 - node index 7 remained collapsed. As a result, the memoizedInsertNode in the newChildren.map() callback trys

1. calling addNodeAtDepthAndIndex with child === Level 0 - node index 0 with no nodes inserted because last position in this node's children is less than the minimum index and there are more children on the level of this node, return without insertion
2. calling addNodeAtDepthAndIndex with child === Level 0 - node index 7 (collapsed) also with no nodes inserted because it has hidden children.
3. end newChildren.map() callback with no nodes inserted hence failed the 'insertedTreeIndex' in mapResult check and ultimately causes memoizedInsertNode to throw new Error('No suitable position found to insert.')

This condition only happens when the dragged node (ie Level 1 - node index 4) has children, is positioned right above a collapsed node like the layout above and moved just enough to start the drag but not enough to move into the node above. In some cases, the collapsed node below will automatically expand and therefore will not trigger the exception but in the event when the collapsed node below remain collapsed, it throws an unhandled exception. I still believe this should not remain an unhandled exception and should be trapped in a try/catch block as @rrhvella proposed as a valid solution.

[datoslabs]

Sharing an update after extensive debugging. I have a setup that allows me to easily reproduce this error, the tree (with custom canDrop function) is as follows:

Level 0 - node index 0
    Level 1 - node index 1
        Level 2 - node index 2
        Level 2 - node index 3
    Level 1 - node index 4 (dragged node)
        Level 2 - node index 5
        Level 2 - node index 6
Level 0 - node index 7 (collapsed)
    Level 1 - node index 8 (hidden)

When Level 1 - node index 4 is dragged slightly upwards to start drag but remain hovering around the original location (or slowly try to move up 1 spot), it will trigger calls to canDrop which in turn calls insertNode with either minimumTreeIndex:3 (ie dropTargetProps.listIndex:3) or minimumTreeIndex:6 (ie dropTargetProps.listIndex:6) with 6 being an invalid minimumTreeIndex. With minimumTreeIndex:6 the newChildren.map callback in addNodeAtDepthAndIndex method loops through [Level 0 - node index 0, Level 0 - node index 7 (collapsed)] as follows:

1. call addNodeAtDepthAndIndex with Level 0 - node index 0, this ended up returning without inserting a node because last position in this node's children is less than the minimum index and there are more children on the level of this node
2. call addNodeAtDepthAndIndex with Level 0 - node index 7 (collapsed), this ended up returning without inserting a node because it has hidden children
3. returns back to insertNode/memoizedInsertNode call without inserted nodes and throws Error('No suitable position found to insert.')

I have observed that the collapsed node Level 0 - node index 7 (collapsed) would auto-expand, if node is dragged far enough down to trigger the expansion, at which point the memoizedInsertNode call with minimumTreeIndex:6 will return successfully with a node inserted.

It appears the bug is either with react-dnd's DragDropMonitorImpl and/or DropTargetMonitorImpl (ie monitor.isOver() returned true prematurely and/or the DropTarget index value is incorrectly determined), or with the logic/timing that is supposed to auto-expand the collapsed node Level 0 - node index 7. In any case, I believe the error should be gracely handled instead of leaving it as an unhandled exception; the simple addition of try/catch and return false in the catch clause proposed by @rrhvella in the canDrop method surrounding memoizedInsertNode has little to no known undesirable side-effects.

[vitrinh282]

did you fix it ?