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

Star handlers called without the event type as the first arg #2

Closed
oaleynik opened this issue Jan 15, 2017 · 12 comments · Fixed by #19
Closed

Star handlers called without the event type as the first arg #2

oaleynik opened this issue Jan 15, 2017 · 12 comments · Fixed by #19
Labels

Comments

@oaleynik
Copy link

oaleynik commented Jan 15, 2017

Looks like all star handlers should be called with type as first argument, aren't they?

image

@oaleynik oaleynik changed the title Star handlers should be called with event type as the first arg Star handlers called without the event type as the first arg Jan 15, 2017
@developit
Copy link
Owner

heh! I only realized after publishing that I'd made the wrong call there but then tweeted the right one. Thanks for the PR.

@developit developit added the bug label Jan 16, 2017
@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 16, 2017

just sorry for the offtopic... @oaleynik which is that theme that you are using? I'm new to Atom and trying to find some meaningful theme for syntax.

@developit
Copy link
Owner

@tunnckoCore that's actually my editor - I commented with the setup in this issue (apparently people really like my editor setup haha).

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 16, 2017

Haha, great & thanks. I'll try the syntax theme. Few weeks on Atom and can't find meaningful theme, tried dozen but all are absolutely awful.

@oaleynik
Copy link
Author

oaleynik commented Jan 17, 2017

@tunnckoCore yeah, that screenshot is the ctrl+c/ctrl+v from Twitter :)
Currently I use https://github.com/apex/apex-ui and https://github.com/apex/apex-syntax created by legendary @tj - I found it super "disruption free" because of absence of the color "noise."

Some other themes I like include:

  • Chester
  • Github Atom Light
  • Material
  • Gloom
  • One Dark Vivid

It of course depends on the mood, time of the day when I work, moon phase, etc.. :)

@developit very sorry for the offtopic :)

@developit
Copy link
Owner

One Dark Vivid for life (though now I'm looking at Gloom)

@tunnckoCore
Copy link
Collaborator

@oaleynik holy shit, so clean and enough, I'll try it, because screenshot is not enough for me :) Thanks again.

So TJ is still so amazing person, haha.

very sorry for the offtopic :)

meh.. it happens some times 😆

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 17, 2017

Btw, a bit on topic.

What about that (fixes #2 - this, resolves #12, 197b and in bonus: you can access the all)

let ret = {
  all: {},
  on (type, handler) {
    list(type).push(handler)
    return ret
  },
  off (type, handler) {
    let e = list(type)
    e.splice(e.indexOf(handler) >>> 0, 1)
    return ret
  },
  emit (type, event) {
    list(type).map((f) => f(event))
    list('*').map((f) => f(type, event))
    return ret
  }
}

let list = (type) => ret.all[type = type.toLowerCase()] || (ret.all[type] = [])

module.exports = () => ret

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 17, 2017

Damn, I'm god? 🚀 179b with #8

let mitt = {
  all: {},
  on (type, handler) {
    list(type).add(handler)
    return mitt
  },
  off (type, handler) {
    list(type).delete(handler)
    return mitt
  },
  emit (type, event) {
    list(type).forEach( f => f(event))
    list('*').forEach( f => f(type, event))
    return mitt
  }
}

let list = (type) => mitt.all[type = type.toLowerCase()] || (mitt.all[type] = new Set())

export default () => mitt

usage

var mitt = require('./dist/mitt')
var ee = mitt()

ee
  .on('*', (type, arg) => console.log('wildcard:', type, arg))
  .on('foo', (arg) => console.log('foo1:', arg))
  .on('foo', (arg) => console.log('foo2:', arg))
  .on('bar', (arg) => console.log('bar:', arg))
  .emit('foo', 123)
  .emit('bar', 444)

edit: so we freely can implement multiple args - we have huge room :D

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 17, 2017

So in short. Can I PR with fixing #2, #3, #12, #13 and bonus accessing all with final 200b exactly? Then you can update the build process and package.json things.

@developit
Copy link
Owner

@tunnckoCore your implementation returns the same instance for every mitt() call, which means listeners are shared for all instances. It needs to return a new instance for each mitt() call so it's not just a singleton.

@tunnckoCore
Copy link
Collaborator

Hm. Actually, yea. I get some tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants