Database listeners may fail to remove, doc update needed

[dlcole]

I have encountered two scenarios that can cause database listeners (such as 'child_changed') to fail to remove.

Here's the code at the end of the on method :

    callback['__fbHandle'] = handle;
    callback['__fbEventType'] = eventType;
    callback['__fbContext'] = context;

    this._handles.set(callback, handle);

this in this case is the path reference, such as firebase().database().ref('user/data').

Consequently, this code will work correctly:

    const callback = function(snapshot) { console.log('callback: ' + snapshot.val()); }
    const ref = firebase().database().ref('user/data');
    const listener = ref.on('child_changed', callback);
    ref.off('child_changed', listener);

Whereas this code will fail to remove the listener:

    const callback = function (snapshot) { console.log('callback: ' + snapshot.val()); }
    const listener = firebase().database().ref('user/data').on('child_changed', callback);
    firebase().database().ref('user/data').off('child_changed', listener);

Because the off method references the handle saved by the on method, but above you have a different instance of the reference.

Here's the entirety of the off method:

    off(eventType?: EventType, callback?: (a: DataSnapshot, b: string) => void, context?: Record<string, any>): void {
        const handle = callback?.['__fbHandle'];
        const event = callback?.['__fbEventType'];
        if (handle && event === eventType) {
            if (this._handles.has(callback)) {
                this.native.removeEventListener(handle as any);
                callback['__fbHandle'] = undefined;
                callback['__fbEventType'] = undefined;
                callback['__fbContext'] = undefined;
                this._handles.delete(callback);
            }
        }
    } 

In the failing case, this references two different objects, and thus this._handles.has(callback) resolves to false and the listener is not removed.

This can be resolved by creating the reference first, then using that same reference for both the on and off invocations, as shown in the success example above.

The second scenario is when a common callback (event handler) is used. In the off method above, the handle, event type, and context all are deleted from the callback when the listener is removed. If you use that same callback function on a subsequent off call, the handle will resolve to undefined and the block that removes the listener will be skipped.

This can be resolved by invoking the common callback within a unique outer function, such as

    const commonCallback = function (snapshot) { console.log('commonCallback: ' + snapshot.val()); }

    const callback = function (snapshot) { commonCallback(snapshot); }
    const ref = firebase().database().ref('user/data');
    const listener = ref.on('child_changed', callback);
    ref.off('child_changed', listener);

I recommend revising the code example in the database readme topic Remove-a-reference-event-listener, and adding a note that a unique callback function must be used with each on invocation.