Return unsub function from on()

[ajoslin]

195b. Forked from #1.

• Don't assign all[type] in .off()
• Use a slightly more space-efficient assignment ofall[type] in .on() (no parens).
• Return a bound splice from .on().

The one thing I can't figure out is the why the "off() should normalize case" test is failing now. In fact, I don't see how it ever worked.

[tunnckoCore]

I believe that's intentional.

[developit]

the assignment isn't needed, just it produced a smaller output size because it is duplicated and thus cheap when gzipped. I'd be curious to see the size with & without this change.

[tunnckoCore]

We still not consider #1, so this will be delayed too...

I'm still strongly against that. Since we promote to be "compat" with Node EE. We still are totally not compat in any terms, but yea..

[developit]

this fails if you remove an earlier-inserted event handler, because it shifts the indices:

let e = mitt(), a=()=>{}, b=()=>{};

e.on('a', a);
e.on('a', b);

e.off('a', a);
e.off('a', b);  // attempts to remove all['a'][1], index is incorrect

AFAIK there is no way around this except using a closure, which is why the other PR was too large.

[ajoslin]

Good point, my "late night coding" brain missed how brittle the splice solution was.

[ajoslin]

Alright, I just pushed. if we replace Object.create(null) with {} at the beginning, we can fit a bound off in 193b. I'm still getting the weird failing test about off normalizing case, though.

Regarding this not being compatible with EE.. This library isn't at all compatible with EventEmitter at the moment. .off() does not exist in EventEmitter, for example. I'd say it's in the "spirit" of EventEmitter, but no more.

[tunnckoCore]

object create is intentional. and yea exactly wat i mean

[ajoslin]

I'm guessing the purpose of Object.create(null) here is to allow event names using object prototype names? Is that correct?

[tunnckoCore]

Yes. Wanted and merged.

In reality we can go down to ~160-170 if we drop off and make on return unsubscribe. If i remember i tried it already. If that's the way to go with mitt, i'm giving up and going to make another lib.

edit: If we are more strict we even can remove the method names and be a single method :D

[tunnckoCore]

What about this one then

function dush (all) {
  all = all || Object.create(null)
  return function onOrEmit (name, handler) {
    if (typeof handler === 'function') {
      var e = (all[name] || (all[name] = []))
      e.push(handler)

      return function off (name) {
        (name && (all[name] = [])) || e.splice(e.indexOf(handler) >>> 0, 1)
      }
    }
    all[name].map((f) => { f(handler) })
  }
}

example

var emitter = dush()

// .on
emitter('foo', (data) => console.log('one', data))
var offBar = emitter('bar', (data) => console.log('bar1', data))
emitter('bar', (data) => console.log('bar2', data))
var offFoo = emitter('foo', (data) => console.log('two', data))

// .emit
emitter('foo', 123)

// off all foo
offFoo('foo')

emitter('foo', 'not emitted')
emitter('bar', 'woohoo')

// off all bar
offBar('bar')

emitter('bar', 33)
emitter('foo', 33)

[developit]

That looks a lot like the observed value syntax from Mithril. Separate lib tho :P

Think we should rename off? You're right about EE compat, I took that name from Wildemitter

[vesparny]

Returning an unsubscribe function from the subscription is the same approach I've been using in brcast.
https://github.com/vesparny/brcast/blob/b5834dc95faad93809c26df54729fba5bfef4e67/index.js#L25

When having a lot of handlers subscriptions/unsubscriptions this implementation is apparently heavier in terms of memory consumption. Probably because a function is allocated every time a subscription is created.

This has been reported to the styled-components repository which uses a similar implementation.

@developit what do you think?

[developit]

^ Yes