Dragging tab around throws during render (clears the screen) under certain conditions [Reproducible in drop-mode example]

[tjcouch-sil]

Hello again!

I ran into a bug on rc-dock@3.3.2 in the project I'm working on. I'm on Windows 11.

When you use dropMode="edge" and an operating system display scale other than 100% and drag a tab between two panels, the following error is thrown on this line in DockLayout.setDropRect:

Uncaught TypeError: Cannot read properties of null (reading 'source')

Repro steps:

1. Open the drop-mode example in maximized browser on a screen at 125% display scale (my laptop screen and monitor both worked at this scale, but it seems neither cause the error at 100% scale)
2. Grab the “Drop Mode” tab
3. Drag it up and down across its border with the panel under it until it throws

Before throw (drag it up and down across the border):

After throw:

It seems to me that DockLayout.setDropRect is getting triggered multiple times in the same moment with direction set to remove and this.state.dropRect is defined. So it queues up a state change to set dropRect to null under certain conditions including checking state.dropRect.source. But on the second trigger of this queued state change, dropRect is already set to null from the previous trigger of this queued state change, so it throws. My simple, under-informed solution is to check for dropRect being defined in the queued state change.

I discovered that the two places that run setDropRect with direction remove when the error occurs are in DockDropEdge.onDragOver and then DockDropEdge.onDragLeave. I didn’t dig in enough to understand why this particular situation causes onDragOver to trigger in addition to onDragLeave. In the problem case, onDragOver is triggered, then onDragLeave is triggered. In normal situations, I see that only onDragLeave is triggered.

I suspect this problem where onDragOver is unexpectedly running has been happening for a long time, but this change surfaced the problem as this error.

Here is the diff that solved my problem:

diff --git a/node_modules/rc-dock/es/DockLayout.js b/node_modules/rc-dock/es/DockLayout.js
index bac288d..6e25d88 100644
--- a/node_modules/rc-dock/es/DockLayout.js
+++ b/node_modules/rc-dock/es/DockLayout.js
@@ -316,7 +326,21 @@ export class DockLayout extends DockPortalManager {
         if (dropRect) {
             if (direction === 'remove') {
                 this.setState((oldStates) => {
-                    if (oldStates.dropRect.source === source) {
+                    /**
+                     * patch-package notes:
+                     *
+                     * Occasionally on a display with non-100% display scale, `DockDropEdge`'s
+                     * `onDragOver` would also fire right before the usual `onDragLeave` when you
+                     * drag a tab between two panels, causing this `setState` function to queue up
+                     * twice in a row, meaning `oldStates.dropRect` would be `null` on the second
+                     * run. As a result, accessing `oldStates.dropRect.source` would throw. I'm not
+                     * sure of the consequences of running `setDropRect` with `remove` twice in a
+                     * row, but it doesn't seem to cause problems right now. As such, quick fix is
+                     * checking if `oldStates.dropRect` exists.
+                     */
+                    if (oldStates.dropRect && oldStates.dropRect.source === source) {
                         return { dropRect: null };
                     }
                     return {};

Does this seem like a reasonable solution? I suspect another wise move would be to figure out why onDragMove is being triggered sometimes in these conditions and stop it, but I suspect this solution is also important since oldStates.dropRect could potentially be set to null from multiple places at the same time; I just happened to run into one case.

As always, thanks for your hard work on rc-dock :)

This issue body was partially generated by patch-package.

[Adamatoma]

Omg, I was just judged by my client about this bug......
When will it be published??

[AllenCyhCHAN]

@tjcouch-sil was there any console error thrown in this case. ?

[tjcouch-sil]

@AllenCyhCHAN yes. This error is thrown:

Uncaught TypeError: Cannot read properties of null (reading 'source')

[tjcouch-sil]

@Adamatoma I don't know. I have decided I will not make a PR yet because I don't have confirmation that this is the direction the author wants to take. I may change my mind and make a PR eventually. For now, though, you can make the same change I made as described in the issue description using patch-package. :)

[AllenCyhCHAN]

Im having the same issue. 🆙

[galushsen]

I have the same issue, still relevant.