on should be idempotent

[jfirebaugh]

Like addEventListener, Evented#on (and Map#on with delegation) should be idempotent, and only do something if there wasn't already a listener for the given event type and callback.

For example, the sequence on('a', cb), on('a', cb), off('a', cb) should leave no cb's listening for a.

[areichman]

@jfirebaugh Using your example, if I don't call off('a', cb), I'm seeing on('a', cb) fire twice. Is that your expectation at the moment?

I'm seeing this behavior while trying to implement a style switcher. I'm calling map.setStyle() then reloading my custom sources and layers (using React state to remember which should be loaded). Some of my sources and layers add their own listeners (e.g. display popup on click), so I get duplicate listeners each time I switch styles.

[jfirebaugh]

Yeah, I believe double firing is what will currently happen if you bind the same function instance to the same event multiple times. But what you're seeing isn't necessarily this issue. A common mistake is to accidentally bind multiple different functions instances (which happen to do the same thing), e.g. with an inline function (on('a', function(e) { ... })), arrow function (on('a', (e) => { ... })), or bind result (on('a', this.onFoo.bind(this))). Since in each of these cases it's a different function instance being passed to on each time, they won't be affected by what's proposed here.

[areichman]

In my case, it is a binded (bound?) function, but that's happening in the constructor of my class:

this.update = this.update.bind(this);

Then I create the event listener using:

map.on('click', this.update);

So I would expect the same function instance to be passed each time, correct? Without a fix for this issue in place, I can look at internally tracking whether the event listener has been created already.

[jfirebaugh]

So I would expect the same function instance to be passed each time, correct?

Yes, provided there's only one instance of the class.