getndazn / kopytko-framework

A modern Roku's Brightscript framework based on JS world's solutions
MIT License
19 stars 6 forks source link

fix: clearing EventBus registered events on component destroy #43

Closed RadoslawZambrowski closed 1 year ago

RadoslawZambrowski commented 1 year ago

What did you implement:

Currently, when the Kopytko component is destroyed, listeners are detached from the globally stored EventBus node. However, data is not removed from the private fields of the EventBusFacade. It may lead to memory leaks.

How did you implement it:

To avoid memory leaks, I introduced a new clear function in the EventBusFacade which is called when the Kopytko component is destroyed. The function removed all data related to the registered events within the scope of the destroyed component.

I also renamed the private _eventBus field to $$eventBus to make it more unique and distinguish it from the regular private field. This is for ensuring unambiguity when checking for EventBusFacade instance existence in the destroyKopytko function and avoid potential naming collisions.

How can we verify it:

There should be no regression when it comes to the EventBus.

No memory leaks on component destroy.

Before changes, memory leaks may occur when an event is registered with the context. For example create APrototype inside a Kopytko component, then destroy the component. Verify memory leaks.

function APrototype(componentNode as Object) as Object
    _constructor = function(m as Object) as Object
        m._eventBus.on("anEvent", m._aHandler, m)

        return m
    end function

    prototype = {
        _componentNode: componentNode
        _eventBus: EventBusFacade()

        aHandler: sub(payload as Object)
        end sub
    }

    return _constructor(prototype)
end function

Todos:

Is this ready for review?: YES Is it a breaking change?: NO