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

Unable to unsubscribe from event listeners #193

Closed
hutch120 opened this issue Aug 18, 2018 · 6 comments · May be fixed by #194
Closed

Unable to unsubscribe from event listeners #193

hutch120 opened this issue Aug 18, 2018 · 6 comments · May be fixed by #194

Comments

@hutch120
Copy link

Hi All,

I've just spent quite a while tracking down an issue that results in multiple triggering of the on('xxxx') functions.

I have a react application that moves between pages, and loads/unloads components as it should. But, when I unload this module, it maintains the listener state, despite calling removeControl.

Essentially the issue (other than annoyance of multiple callbacks) is that eventually you will hit the maxListeners limit of 10 if you call the on handlers. E.g. on('route') because there is no way to unregister listeners, not even by using removeControl.

I've got a workaround in my code that sets a static variable, and checks that before registering the listener, but feelsbadman.

I think there are two things that could resolve this.

  1. Remove listeners on removeControl
{
    key: 'onRemove',
    value: function onRemove(map) {
      this._events[type] = null; // <<====== ADD THIS LINE

and/or

  1. Add an un function to remove a listener. A bit harder..
{
    key: 'un',
    value: function on(type, fn) {
      this.actions.eventUnSubscribe(type, fn);
      return this;
}
function eventUnSubscribe(type, fn) {
  return function (dispatch, getState) {
    var _getState8 = getState(),
        events = _getState8.events;

    events[type] = events[type] || [];
    // >>>>> FIND AND REMOVE THE EVENT <<<<<
    return {
      type: types.EVENTS,
      events: events
    };
  };
}
@patrix
Copy link

patrix commented Aug 23, 2018

I had the same problem, then I choose the second option.

The issue #40 have a commit related that implements the off method like the your un function.

@hutch120
Copy link
Author

@patrix Thanks for referencing that code. I'll close this issue. For reference, my workaround in React looks something like this:

class MapComponent extends React.Component {
  static routeListenerAdded

  componentDidMount () {
    this.mbControlDirections = new window.MapboxDirections({ ... })
    if (!MapComponent.routeListenerAdded) {
      MapComponent.routeListenerAdded = true
      this.mbControlDirections.on('route', evt => { ... })
    }
  }

  componentWillUnmount () {
    if (this.mbControlDirections) {
      this.mbMap.removeControl(this.mbControlDirections)
    }
  }
}

patrix added a commit to patrix/mapbox-gl-directions that referenced this issue Oct 24, 2018
Fixes mapbox#193
Inspired by tristen commit ref mapbox#40
patrix added a commit to patrix/mapbox-gl-directions that referenced this issue Nov 28, 2018
Fixes mapbox#193
Inspired by tristen commit ref mapbox#40
@neeleshbisht99
Copy link

Can you please elaborate the second method a bit more ?

@hutch120
Copy link
Author

@neeleshbisht99 patrix coded it... not sure how much clearer it can be than that... #194

@neeleshbisht99
Copy link

@hutch120 I got it , thanks

@douglasg14b
Copy link

douglasg14b commented Jan 15, 2022

It's still ambiguous, it doesn't seem to comment on how it guarantees function matches. For instance, does it unsubscribe:

 map.on('zoom' () => handleZoom());

map.off('zoom' () => handleZoom());

My understanding is that this might now? If not, then that's a gotcha that isn't made clear.

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.

4 participants