Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error in _onResizeHandler -> listener not removed correctly on destroy() #1369

Closed
mtveerman opened this issue Sep 21, 2020 · 1 comment · Fixed by #1377
Closed

error in _onResizeHandler -> listener not removed correctly on destroy() #1369

mtveerman opened this issue Sep 21, 2020 · 1 comment · Fixed by #1377

Comments

@mtveerman
Copy link

Subject of the issue

I have a SPA in Vue where I create and destroy grids when switching pages. All seems to work fine, until a resize event is called. I get errors:
Cannot set property column of undefined, pointing to https://github.com/gridstack/gridstack.js/blob/develop/src/gridstack.ts#L503
or
Cannot read property '_max' of null, pointing to
https://github.com/gridstack/gridstack.js/blob/develop/src/gridstack.ts#L1102

Both of these originate from the _onResizeHandler and I believe the problem is there. The event listener is not removed from the window during a destroy of the grid. Therefore the listener remains active, but when called, an error occurs because the grid is no longer there.

The listener is added using a .bind(this) here: https://github.com/gridstack/gridstack.js/blob/develop/src/gridstack.ts#L305
and removed inside the destroy function here: https://github.com/gridstack/gridstack.js/blob/develop/src/gridstack.ts#L541

But in reality it's not removed, and one can see in the dev pane of the browser. Reason is because of the .bind as explained here: https://stackoverflow.com/a/22870717/9258054

Your environment

^2.0.0
Chrome 85

Steps to reproduce

https://jsfiddle.net/fwgpyhs8/1/
Although I cannot reproduce the error in the console, this example does show that the event listener remains active even thought the grid was destroyed.

Expected behaviour

Event listener should be removed on destroy()

Actual behaviour

Event listener remains active, leading to errors when actual event is called.

adumesny added a commit to adumesny/gridstack.js that referenced this issue Sep 26, 2020
fix gridstack#1369, we now remove resize event correctly in destroy()
@adumesny
Copy link
Member

adumesny commented Sep 26, 2020

thank you very much for finding this (which I've ran into but ignored until now) with a solution. Will be in 2.0.1 (soon)

adumesny added a commit to adumesny/gridstack.js that referenced this issue Sep 26, 2020
fix for gridstack#1369
* private _onResizeHandler() is now onParentResize() as client might want to manually call this
(when dome div above resizes)
* we now store the event binding method to we can not just remove the handler
BUT also delete it to free class pointer
(else GridStack classs will likely not free up)
* also only top grid now register size event, children get it through parent (which happens during cell resize as well)
* only register window resize if we need to as well
adumesny added a commit to adumesny/gridstack.js that referenced this issue Sep 26, 2020
fix for gridstack#1369
* private _onResizeHandler() is now onParentResize() as client might want to manually call this
(when dome div above resizes)
* we now store the event binding method to we can not just remove the handler
BUT also delete it to free class pointer
(else GridStack classs will likely not free up)
* also only top grid now register size event, children get it through parent (which happens during cell resize as well)
* only register window resize if we need to as well
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants