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

fix: call star handlers with type as first parameter #3

Closed
wants to merge 1 commit into from

Conversation

oaleynik
Copy link

Closes #2

@developit
Copy link
Owner

Awesome, thanks Oleg!

@fregante
Copy link

Shouldn't it be the second parameter? I'd expect the first parameter to be the same regardless of how the event it's registered.

@dmitriyzhirma
Copy link

dmitriyzhirma commented Jan 17, 2017

It could be better like this:

...
emit(type, ...args) {
 const data = { type: type, args: args };
 list('*').concat(list(type)).forEach(f => f(data));
}
...

Usage:

import mitt from 'mitt';

const emitter = mitt();

emitter.on('*', ({type, args}) => {
 console.log(type);
 args.forEach(a => console.log(a));
});

emitter.on('auth', ({type, args}) => {
 console.log(type);
 args.forEach(a => console.log(a));
});

emitter.emit('auth', {a: 'b'}, {c: 'd'});
emitter.emit('success', 'foo', 'bar');

@developit
Copy link
Owner

Currently only one argument is supported.
@bfred-it semantically yes. I've found a way to make it fit within the 200b budget: 3a675c7#diff-1fdf421c05c1140f6d71444ea2b27638R38

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 17, 2017

@developit, in first glance it not looks good. But I review the full smaller branch and it looks okey.

And actually, why not just support #13 multiple arguments

emit(type, a, b, c, d) {
  (all[type = type.toLowerCase()] || []).map( f => { f(type, a, b, c, d); });
  return ret;
}

yea it will lowercase when * but.. it worth and not so big cost, imho?

edit: btw, add .editorconfig please

@developit
Copy link
Owner

@tunnckoCore will add .editorconfig, noticed it was missing yesterday.

The issue with your suggestion is that it adds type to every event, not just *. I'm open to the minimal argument forwarding, just it needs to be accounted for in the filesize some other way. Currently that branch is sitting at 203 bytes because of the change in how size is measured.

@tunnckoCore
Copy link
Collaborator

The issue with your suggestion is that it adds type to every event

Actually yea, realized that after few moments ;d just didn't sleep last night.

@developit
Copy link
Owner

Fixed in #19

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 this pull request may close these issues.

5 participants