DataTable: 10.8.3 - Drag selection fails due to TypeError: event.originalEvent is undefined (Next.JS)

[SolidAnonDev]

Describe the bug

Drag selection fails and is in general having problems across version 10.8.2 and 10.8.3, but appears to be entirely broken in 10.8.3 in Next.JS

See #7192 for previous bug report regarding 10.8.2

Reproducer

https://stackblitz.com/edit/stackblitz-starters-zx3txq?file=package.json

System Information

System:
    OS: Linux 5.0 undefined
    CPU: (4) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 18.20.3 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 10.2.3 - /usr/local/bin/npm
    pnpm: 8.15.6 - /usr/local/bin/pnpm
  npmPackages:
    primereact: ^10.8.3 => 10.8.3 
    react: ^18.3.1 => 18.3.1 
    tailwindcss: ^3.4.11 => 3.4.11 


"dependencies": {
    "@types/node": "^22.5.5",
    "@types/react": "^18.3.6",
    "@types/react-dom": "^18.3.0",
    "autoprefixer": "^10.4.20",
    "eslint": "^8.57.0",
    "eslint-config-next": "^14.2.11",
    "next": "^14.2.11",
    "postcss": "^8.4.47",
    "primereact": "^10.8.3",
    "react": "^18.3.1",
    "react-dom": "^18.3.1",
    "tailwindcss": "^3.4.11",
    "typescript": "5.2.2"
  }

Steps to reproduce the behavior

1. In reproduction link, attempt to make a drag selection across datatable rows
2. An error will be thrown:

Unhandled Runtime Error

TypeError: event.originalEvent is undefined
Call Stack
event
node_modules/primereact/datatable/datatable.esm.js (2630:61)
allowDrag
node_modules/primereact/datatable/datatable.esm.js (2633:37)
event
node_modules/primereact/datatable/datatable.esm.js (3130:22)
onRowDragStart
node_modules/primereact/datatable/datatable.esm.js (2190:11)
_onDragStart
node_modules/primereact/datatable/datatable.esm.js (2419:14)

Expected behavior

A successful drag selection can be made.

[sja-cslab]

@melloware I got the same problem so it's not NextJs related.

I've analysed that a few min and figured out, that if you start in the upper left corner and drag down/right - that error seems not to happen.

In that case

var _onMouseDown = function onMouseDown(event) {
    props.onRowMouseDown({
      originalEvent: event,
      data: props.rowData,
      index: props.rowIndex
    });
  };

is called. However - if you start with one of the other corners and try to drag select not just the _onMouseDown is called but

var _onDragStart = function onDragStart(event) {
    props.onRowDragStart({
      originalEvent: event,
      data: props.rowData,
      index: props.rowIndex
    });
  };

also.

The problem here is the implementation of onRowDragStart which starts as follows:

var onRowDragStart = function onRowDragStart(e) {
    var event = e.originalEvent,
      index = e.index;
    if (allowRowDrag(event))

after the var event = ... the allowDrag function does not receive the original event which causes a crash at

var allowDrag = function allowDrag(event) {
    return props.dragSelection && isMultipleSelection() && !event.originalEvent.shiftKey;
  };

because event.originalEvent isn't defined.

[melloware]

basically it needs event.originalEvent defensive null checking?

[sja-cslab]

That would avoid the error at this point, yes. !event?.originalEvent?.shiftKey; should fix it.

However I'm not sure if thats the right place - just because we would avoid that doesn't mean we fixed it.

The question here is "Why is dragStart called in 3 of 4 cases" i guess

[melloware]

Good question!

[gcko]

hey @sja-cslab @SolidAnonDev @melloware check PR #7217 -》

The main issue is the existing implementation of allowRowDrag takes an "event" and passes it through to allowDrag, however allowDrag expects the event parameter to have originalEvent as a property. This PR fixes that issue.

changing the function

const allowRowDrag = (event) => {
    return (!allowCellSelection() && allowDrag(event)) || props.reorderableRows;
};

to the following

const allowRowDrag = (event) => {
    return (!allowCellSelection() && allowDrag({ originalEvent: event })) || props.reorderableRows;
};

Fixes the issue

[sja-cslab]

@gcko are you pretty sure? While debugging i noticed, that originalEvent is already there

at least in some cases. And that seems to be the problem.

As said before, the problem is caused by the destructuring here:

var onRowDragStart = function onRowDragStart(e) {
    var event = e.originalEvent, // this here is bad for following code
      index = e.index;
    if (allowRowDrag(event)) { // previous desctucture make callchain within here throw

But - and that's what i asked @melloware before is the question, why dragStart is called sometimes.

You can currently start dragselection in top/left and move your mouse down/right - that makes onDragStart not get called.
If you do it the other way, start down/right, and move top/left you get the exception bevause dragStart handler is called.

Or is there something I don't see?

[gcko]

@sja-cslab 🤔 you are right, my "fix" is incorrect, let me look again

[gcko]

Alright @sja-cslab, I removed the change and instead bypass the destructing by passing e to allowRowDrag ->

Regarding onRowDragStart and onDragStart, and onMouseDown ->

The onDragStart method is tied to the native browser event for the each tr element. It proxies to onRowDragStart. onRowDragStart is only called through this proxy from what I see.

onMouseDown method again is tied to the native browser event for each tr, and it proxies to onRowMouseDown. In a similar fashion, onRowMouseDown is only called through the proxy.

It seems like there is a relationship between onMouseDown and onDragStart in relation to dragging - onRowMouseDown has a section as follows:

if (allowRowDrag(e)) {
    enableDragSelection(event, 'row');
    anchorRowIndex.current = e.index;
    rangeRowIndex.current = e.index;
    anchorRowFirst.current = props.first;
}

The enableDragSelection handles the reference for the drag selection helper as well as binding listeners to mousemove and mouseup on the document.

onRowDragStart on the other hand checks if allowRowDrag returns true, and if it does it will update references for rowDragging and draggedRowIndex - these references are then used throughout onRowDragOver and onRowDragLeave

[sja-cslab]

@gcko guess you are absolutely right.

What I don't get is why the drag event is called based on mouse move direction (if thats even the trigger). But that should be analysed in another ticket...

[sja-cslab]

We just got #7221 bet that is related somehow.

Edit:
@melloware @gcko

I just looked around in last merged PRs and found this one.
See diff.

Not sure if that's the reason for that but the changed event.currentTarget.draggable = isDraggableHandle; event.target.draggable = !isDraggableHandle; is a different logic than it was - the previous else {event.currentTarget.draggable = false;} wont happen anymore.

[melloware]

@KumJungMin can you look at what @sja-cslab is saying

[gcko]

@sja-cslab agreed #7221 is related to this issue (and should be fixed by my pr as well)

regarding the diff from the PR you referenced, #7051, That logic makes the Typescript dev in me cringe, however lets break down the change:

// isDraggableHandle can be either a boolean or an HTMLElement or null
const isDraggableHandle = isUnstyled()
// returns boolean | HTMLElement | null
                ? DomHandler.getAttribute(event.target, 'data-pc-section') === 'rowreordericon' || event.target.closest('[data-pc-section="rowreordericon"]')
// returns boolean | HTMLElement | null
                : DomHandler.hasClass(event.target, 'p-datatable-reorderablerow-handle') || event.target.closest('.p-datatable-reorderablerow-handle');

// can be one of boolean | HTMLElement | null
event.currentTarget.draggable = isDraggableHandle;
// can be a boolean (using `!` will coalesce a value to a boolean)
event.target.draggable = !isDraggableHandle;

As you said @sja-cslab, the logic is different. Also because of the coalescing the values can be wrong for event.currentTarget.draggable -> a quick fix would be to do a the following:

// use !! to force it to a boolean
event.currentTarget.draggable = !!isDraggableHandle;

Extra info on the closest() method -> https://developer.mozilla.org/en-US/docs/Web/API/Element/closest