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

Support emitting multiple arguments #13

Closed
tunnckoCore opened this issue Jan 16, 2017 · 8 comments · Fixed by #19
Closed

Support emitting multiple arguments #13

tunnckoCore opened this issue Jan 16, 2017 · 8 comments · Fixed by #19

Comments

@tunnckoCore
Copy link
Collaborator

Just like .emit('foo', 1, 2, 3). Initially thought for rest + spread but it adds around 50 bytes.
So I think it may be enough to have explicitly defined 3-4 arguments

That adds 4 bytes.

    emit(type, a, b, c) {
      list('*').concat(list(type)).forEach( f => { f(a, b, c); });
      return ret;
    }

Some faster implementations than node core's uses this tactic, but up to 6-8 args

@tunnckoCore
Copy link
Collaborator Author

tunnckoCore commented Jan 17, 2017

So finally, I just tried to see what I'll get if I merge everything ( #8, #11, #12 + this one #13 ) and it is 210 bytes which looks good number :P :)

@developit
Copy link
Owner

developit commented Jan 17, 2017

Nice! Though I might have an interesting thought that changes your mind about the multiple arguments thing from @bevacqua:
screen shot 2017-01-16 at 7 21 17 pm

@tunnckoCore
Copy link
Collaborator Author

hmm.. yep, forgot about that

@Aaronius
Copy link

FWIW, I would use support for multiple arguments before using *.

@developit
Copy link
Owner

You might be surprised how convenient * is! Some uses I've had for it over the years:

  • logging (example: on('*', console.log))
  • refiring modified or aggregate events (example: on('log:debounced'))
  • collating events between two emitters (example: inside/outside a worker)
  • implementing rudimentary event bubbling (listen to child's *, refire if e.bubble)
  • even implementing case-insensitive event handling 💃

@tunnckoCore
Copy link
Collaborator Author

I'm absolutely agree. Working (emitting) only one argument is cool and very clean. So yea, I can agree to remove that extra second argument from #19, but we should mention that in the docs that emit can only emit one argument. :)

@bevacqua
Copy link

You can easily do both.

  • .emit(type, ...rest)
  • .on(type, (type, ...rest) => {})
  • .on('*', (type, ...rest) => {})

@tunnckoCore
Copy link
Collaborator Author

Yea. Closing per f4c60f4

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