fix: "Invariant Violation: Expected targetIds to be registered."

[joshkel]

We've sporadically seen this error from customers. The call stack indicates that it's originating from HTML5BackendImpl's
handleTopDragOver's requestAnimationFrame callback. I've been unable to reproduce it locally; however, if I simulate a slowdown by replacing requestAnimationFrame(callback) with setTimeout(callback, 10000), I can fairly reliably reproduce this error.

To fix it, I believe HTML5BackendImpl should consistently clear the hover animation whenever the drag operation is ended for any reason.

I locally tested this fix in the v15.1.2 tag, and it appeared to work.

Fixes #763, #3403

[vsiao]

Thanks for trying to fix this bug! I just commented on https://github.com/react-dnd/react-dnd/pull/3313/files#r836932854 and I think it's still relevant here so I'm cross-linking it. From what I can tell, it still seems very possible to encounter bad race conditions, even with the changes that you've made.

Did you try always calling cancelHover before scheduling to ensure that we're at least capturing the most recent copy of dragOverTargetIds?

[joshkel]

Thanks for the feedback, @vsiao. So, if I'm understanding correctly:

In the original code, if the user hovers over element A, then quickly hovers over element B before the requestAnimationFrame for A can fire, then the hover will be out of date.

After my changes, I think this won't throw errors, because I think the code is structured to cancel any hover effects before anything that would invalidate a target ID. (Actually, it may be possible for an externally triggered rerender to invalidate a target ID. That may need more investigation.) But it's still a visual bug.

To address that, you're saying we should also cancel the hover effect before triggering any hover effect?

That seems correct to me (and so @darthtrevino's fix should go outside of the if, like you said).

[codecov]

Codecov Report

Merging #3409 (f8ba3ee) into main (0830641) will decrease coverage by 0.22%.
The diff coverage is 0.00%.

❗ Current head f8ba3ee differs from pull request most recent head c901c10. Consider uploading reports for the commit c901c10 to get more accurate results

@@            Coverage Diff             @@
##             main    #3409      +/-   ##
==========================================
- Coverage   55.40%   55.18%   -0.23%     
==========================================
  Files         203      203              
  Lines        3238     3251      +13     
  Branches      582      585       +3     
==========================================
  Hits         1794     1794              
- Misses       1444     1457      +13     

Impacted Files	Coverage Δ
packages/backend-html5/src/HTML5BackendImpl.ts	13.35% <0.00%> (-0.57%)	⬇️

[vsiao]

@darthtrevino this fix that you added is always a no-op since it's inside the if condition—it should be moved before the if.

[darthtrevino]

Gotcha, thanks - is this even necessary then?

[omridevk]

@darthtrevino any idea on when a new release with this fix is going to be released?
Appreciate the very awesome library, but this is currently a real show stopper for us.

[omridevk]

Also, I checked locally and I still encounter this issue when trying to use with react-virtualized.

[joshkel]

@omridevk, if you can share a test project or a codesandbox.io demo, I can investigate further.

[omridevk]

i'll try to, it just a private project that I am working on, I'll try to create a minimal reproduction example.

[RobinKamps]

Unfortunately this error still persists - it occurs on nested lists, most times if you drag around like crazy.
Here is an angular :-) example:

Demo:
https://stackblitz.com/github/RobinKamps/dnd-test

Repo:
https://github.com/RobinKamps/dnd-test

[miracle0930]

Not sure if this is a correct fix. Currently, monitor.isOver() may be false even you have already hovered into it. Seems that some requestAnimationFrame is not canceled then the scheduleHover mistakenly dispatches a hover action with empty targetIds to the reducer. I tried and found that adding one more cancelOver at the end of handleTopDragEnter will resolve this issue.