gridstack / gridstack.js

Build interactive dashboards in minutes.
https://gridstackjs.com
MIT License
6.53k stars 1.27k forks source link

Error in handleChange with Undefined nodes on Aurelia-Gridstack v3.0.0 & v4.0.0 #2608

Closed andreitaranu closed 7 months ago

andreitaranu commented 7 months ago

Note

This issue was originally created on the aurelia-gridstack repository but I've received comment that it belongs here 🙂

Description

Encountering a runtime error in the handleChange method within aurelia-gridstack. The issue is observed both in version 3.0.0 and persists after updating to version 4.0.0. The error occurs when handleChange is triggered with an undefined or empty nodes parameter.

Steps to Reproduce

  1. Use grid-stack in an Aurelia component with bindable properties.
    <template>
    <grid-stack float options.bind="gridOptions" ref="gridStackEl">
        <grid-stack-item //someoptions>
            <input type="date" date.bind="fromDate">
        </grid-stack-item>
    </grid-stack>
    </template>
  2. Select a date and observe that changes in unrelated bindable properties seem to trigger handleChange in grid-stack. image

    image

Expected Behavior

handleChange should not be triggered by changes in unrelated bindable properties, or it should be able to handle undefined nodes without throwing an error.

Actual Behavior

handleChange is triggered with an undefined nodes parameter, resulting in a runtime error.

Additional Context

The issue appears to be related to how Aurelia's binding system interacts with grid-stack. Removing certain bindings (e.g., date.two-way="fromDate") prevents the error, suggesting a possible conflict in change detection or event handling.

Environment

Any insights or suggestions on this issue would be greatly appreciated.

adumesny commented 7 months ago

nothing I can look into without reproducing the issue with plain GridStack code (no framework that may have their own bugs - who I'm suprised it's using so many overrides and duplicating properties already in options - anyway my job isn't to maintain Aurelia) that also uses the latest v10 lib (you're v8.4). re-open when you do provide a running example with bug in GS

JosepBergay commented 6 months ago

Hello, we encountered a similar issue using the latest version.

Here's a repro with plain js.

Basically if a widget (in this case an input) triggers the change event, it will be bubbled up to gridstack.

I'm fine with the behavior but I think GridStackNodesHandler should be typed accounting for nodes being undefined. This way users running TS in strict mode will notice.

I can provide a PR. As I see it, we should only modify types.ts. I don't know the implications though. Let me know what you think.

Awesome lib btw, keep up the good work!

adumesny commented 6 months ago

@JosepBergay how do you make the div cause a change event ? I try typing, pressing return, dragging inside the input. nothing. resizing teh item does send the node[0] over as expected. and no the call should not happen with undefined nodes - don't change the type, fix the cause instead.

JosepBergay commented 6 months ago

It's not a gridstack custom 'change' event, it's the standard html element 'change' event.

We can trigger the event writing in the input and then pressing enter.

It's the input that's causing the event, but once it bubbles, the div also receives it.