Multiple Grids - Draggable destroy being called on non-existent element

[ghost]

Subject of the issue

gridstack.all.js:176 Uncaught Error: cannot call methods on draggable prior to initialization; attempted to call method 'destroy'
at Function.error (gridstack.all.js:176)
at HTMLDivElement. (gridstack.all.js:6010)
at Function.each (gridstack.all.js:197)
at E.fn.init.each (gridstack.all.js:126)
at E.fn.init.b.fn. [as draggable] (gridstack.all.js:6002)
at e.draggable (gridstack.all.js:8550)
at o. (gridstack.all.js:5476)
at Array.forEach ()
at o.removeAll (gridstack.all.js:5473)
at ?login:242

Your environment

gridstack.js 1.1.1
Chrome version 80.0.3987.163 (Official Build) (64-bit)

Steps to reproduce

Create two grids, add stuff to first grid then add stuff to second grid, remove one by one each widget from the second grid then call grid.removeAll() on that second grid, you will get this error as the nodes for that grid is not empty yet the grid it's self has no widgets.

Expected behaviour

What should happen is a check to see if the grid being cleared has remaining elements inside of it and whether or not the engine nodes list matches up or not, if not then empty the engine.nodes array so to match the actual status of the grid element.

Actual behaviour

The second grid is empty after calling grid.removeWidget on each widget but the engine.nodes of the grid in question still contains a node which causes the error to fire.

My solution is to check if the grid is empty using jquery, if it is empty and the grid.engine.nodes isn't empty I empty it using array splice which resolves the issue.
if(grids[1].engine.nodes.length>0 && $(bgGrid.el).is(':empty')) grids[1].engine.nodes.splice(0, grids[1].engine.nodes.length);

This solution is good as even if this is fixed in the future it won't break peoples projects.
It may be important to note in my testing that the main grid (grids[0]) doesn't have this issue, only additional grids which I find a bit odd.

[adumesny]

that is odd why only happens on second grid - so you use grid.removeWidget() on each, then call removeAll() ?
can you post a jsfiddle ? then I can copy/paste that and add it to the unit testing going forward. thanks

[ghost]

I'm not able to recreate it using fiddle or even on the demon for two grids, so I'm not even sure now what the issue even is and why I'm getting it on my setup. I just tried in the browser console on the two grids demo but it doesn't get the error but I'm doing it the same way I think, I'll check some more later when I have some time again and make sure.. again.. that I am not doing something to cause it. I thought I had tested it as many ways I could to rule out my own work but now not so sure seeing I can not recreate it using the gridstack.js in the raw.

[adumesny]

from your description is sounds like the engine.nodes (which still have a an element pointer) are not being removed when calling grid.removeWidget() ? - I would step in and make sure it does... maybe I need to nuke the HTML pointer and not depend on garbage collection from getting it later as that might cause the event from still triggering ? might be a timing issue. I didn't look.
but if you have a repro case step in and see...

dargging widgets out will delay delete them, but calling removeWidget() should be immediate

[ghost]

OK I found the issue and it is with my code after all, it's bit of a large project.
I apparently forgot to update my delete function it was calling the first grid and not the grid it should have for removeWidget.

That said, this highlights an issue where that you can delete a widget from one grid by calling .removeWidget from another grid, this is what causes the error, because you have removed the HTML element by doing so but the node stays in engine.nodes of the grid you wanted to remove from.

You can recreate that behavior by doing something like with the two grids demo
grids[0].removeWidget($('.grid-stack-instance-6637').find('.grid-stack-item').first()[0]);

It will remove the first element from the second grid (in this case grid-stack-instance-6637) but will leave it's node in grids[1].engine.nodes as it tried to remove the node in question from grids[0].engines.nodes

So perhaps a small check to see if the desired widget to be deleted is a member of the grid the .removeWidget function is being called on, if it isn't a member silently fail or post console message stating so.

I am both sorry for reporting what I thought was a bug but hey, it turned out not to really be a bug though highlighted something that could be improved, to bad things didn't always pan out like this!

[adumesny]

so you're doing grids[0].removeWidget( grids[1].engine.nodes[0].el ) and it's (incorrectly) removing the element but leaving a tangling node behind. that should be easy to repro and fix. Please create a fiddle if you can. thank you.

[ghost]

https://jsfiddle.net/v9cj6nae/6/

Done!
I think something like this should do the trick though it may not be the best way to do it:

var found = false;
$.each(this.engine.nodes, function(i,v){
    if(v.el==el) found = true;
})
if(!found) return; //return without removing widget as it is not a member of this grid

Where el is the supplied element from the user to the method and this being the owner of the method.

[adumesny]

thanks! we don't use jquery in the upcoming 2.x version - there's also no need here as you can just this.engine.nodes.find()

[adumesny]

fixed in next release. thanks.