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

emit with multiple arguments #36

Closed
rsbondi opened this issue Feb 8, 2017 · 15 comments
Closed

emit with multiple arguments #36

rsbondi opened this issue Feb 8, 2017 · 15 comments

Comments

@rsbondi
Copy link

rsbondi commented Feb 8, 2017

emit(type, ...a) {
  list('*').concat(list(type)).forEach( (f) => { f.apply(null, a); });
}

this seems it would be backwards compatible and support multiple arguments

@developit
Copy link
Owner

Duplicate of #13. The code for this ends up being much too large (...a transpiles to a cloning loop), and it's functionality not supported by Node's EventEmitter, which this library tries to mirror.

@tunnckoCore
Copy link
Collaborator

and it's functionality not supported by Node's EventEmitter

You're wrong here. Node's emitter support emitting multiple arguments, and that was why i suggested it. We just end up that it is enough to be one argument.

And if you need some kind of "multiple args" you can pass an object which is pretty flexible and powerful enough.

@developit
Copy link
Owner

Ahh, I have a very poor memory haha. I wonder if it fits in the 200b budget via your refactor PR (when I get a spare second to merge it 😭)

@tunnckoCore
Copy link
Collaborator

It was exactly 200b (in one point of time, in another it was lower, as it is currently 196b) with everything (few other PRs and issues included).

@rsbondi
Copy link
Author

rsbondi commented Feb 8, 2017

Well then the old fashion way without transpiling should save some bytes

list('*').concat(list(type)).forEach( (f) => { f.apply(null, [].slice.call(arguments, 1)); });

@developit
Copy link
Owner

Yup, definitely smaller than the Babel transpiled version. Still bumps things up to around 230 bytes though 🙊. @tunnckoCore if that was 200b including multiple args then that's pretty sweet.

@tunnckoCore
Copy link
Collaborator

@rsbondi we already tried everything. Best and in same time fast way is to define few arguments (2-3-4) explicitly. All other EEs which are fast in that same time do such thing - they even save the call to apply.

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

It not support infinite args, but it kinda feel good and stay at 200b.

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Feb 8, 2017

@developit actually, my branch currently is exactly 200b without the support for multiple arguments.

It is just a matter of decisions. If we don't support chaining ( #12 ) we could get support for 3 multiple arguments and 199b.

    emit(type, a, b, c) {
      (all[type] || []).map((handler) => { handler(a, b, c); });
      (all['*'] || []).map((handler) => { handler(type, a, b, c); });
    }

@tunnckoCore
Copy link
Collaborator

Actually, with 5 multiple arguments support, the CJS bundle is 202b, IIFE is 199b, with 4 multiple args respectively 201b and 197b. With 3 args support - 199b CJS, 196b is IIFE.

Lastest build

		emit(type, a, b, c, d, e) {
			(all[type] || []).map((handler) => { handler(a, b, c, d, e); });
			(all['*'] || []).map((handler) => { handler(type, a, b, c, d, e); });
		}

output

% 1 ❯ nr build                                                                                         February 09, 00:18:17

> mitt@1.0.1 build /home/charlike/hub/raut/mitt
> npm-run-all clean rollup


> mitt@1.0.1 clean /home/charlike/hub/raut/mitt
> rimraf dist


> mitt@1.0.1 rollup /home/charlike/hub/raut/mitt
> rollup -c


> mitt@1.0.1 postbuild /home/charlike/hub/raut/mitt
> ls -al dist/

total 32
drwxr-xr-x 2 charlike users 4096 Feb  9 00:18 .
drwxr-xr-x 7 charlike users 4096 Feb  9 00:18 ..
-rw-r--r-- 1 charlike users  321 Feb  9 00:18 mitt.iife.js
-rw-r--r-- 1 charlike users  199 Feb  9 00:18 mitt.iife.js.gz
-rw-r--r-- 1 charlike users  312 Feb  9 00:18 mitt.js
-rw-r--r-- 1 charlike users  202 Feb  9 00:18 mitt.js.gz
-rw-r--r-- 1 charlike users  463 Feb  9 00:18 mitt.umd.js
-rw-r--r-- 1 charlike users  267 Feb  9 00:18 mitt.umd.js.gz

using zopfli and uglify plugins, and cleaner npm scripts

@developit
Copy link
Owner

@tunnckoCore there's also this change, which could make the size better or worse, not sure. It switches the default module to a crazy global-or-commonjs thing that isn't UMD.

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Feb 8, 2017

Don't know. I'm using rollup uglify plugin with just opts.compress.warnings: false. Can't get why reserved is needed? We don't need that.

We should look IIFE bundles for the size - they are smallest. We should use plugins for uglify and gzip (using zopfli). And we should stay tuned to ls -al dist - we don't need gzip-size cli and strip json comments.

These 200b is not so real in any way. Users will use at least the UMD bundle, first. Second, sourcemaps are need, so ~30-40b more (for the sourcemap comment). So finally we get ~300b real.

With sourcemaps

drwxr-xr-x 2 charlike users 4096 Feb  9 00:55 .
drwxr-xr-x 7 charlike users 4096 Feb  9 00:55 ..
-rw-r--r-- 1 charlike users  347 Feb  9 00:55 mitt.iife.js
-rw-r--r-- 1 charlike users  227 Feb  9 00:55 mitt.iife.js.gz
-rw-r--r-- 1 charlike users 2239 Feb  9 00:55 mitt.iife.js.map
-rw-r--r-- 1 charlike users  333 Feb  9 00:55 mitt.js
-rw-r--r-- 1 charlike users  224 Feb  9 00:55 mitt.js.gz
-rw-r--r-- 1 charlike users 2233 Feb  9 00:55 mitt.js.map
-rw-r--r-- 1 charlike users  488 Feb  9 00:55 mitt.umd.js
-rw-r--r-- 1 charlike users  294 Feb  9 00:55 mitt.umd.js.gz
-rw-r--r-- 1 charlike users 2238 Feb  9 00:55 mitt.umd.js.map

@developit
Copy link
Owner

Hmm - I don't imagine many people are using UMD.. most would either be using CommonJS (the one measured) or ES Modules (smaller, but unrealistic due to ES syntax).

@tunnckoCore
Copy link
Collaborator

Okey, nevermind. In any way, it is up to you which features to include, but you should choose between "chaining" and "multiple (3-4) args", haha. We can't get both.

@developit
Copy link
Owner

I'm kinda liking the multiple args. BTW after I finally get around to merging your PR I'll just add you as a contributor to the lib. Most of its yours at this point lol

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Feb 9, 2017

Haha, thanks. HOLY SHIT I HAVE WRITTEN WHOLE 200 BYTES !!1!11! 🎉 🤣

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

No branches or pull requests

3 participants