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 chaining #12

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

Support chaining #12

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

Comments

@tunnckoCore
Copy link
Collaborator

tunnckoCore commented Jan 16, 2017

It would be cool and is already some habit for some devs.

const mitt = require('mitt')
const emitter = mitt()
emitter.on('foo', console.log).on('bar', console.log).emit('foo', 123)
@developit
Copy link
Owner

Interesting! Would be curious to see the filesize hit.

@tunnckoCore
Copy link
Collaborator Author

tunnckoCore commented Jan 16, 2017

I think the easiest fix is to make to exports. The default to be the "instance" (like invoking mitt()) and the another to export the current function

export default mitt()
export const mitt

function mitt(all) {

// code ...

Something like that? So if users want to add initial events through mitt({}) they will use

import { mitt } from 'mit'

otherwise they will have directly all the methods

import emitter from 'mitt'

emitter.on().on()

@tunnckoCore
Copy link
Collaborator Author

Oookey, okey, let me think a bit, i'll try. Coding at github issue isn't feels good haha

@developit
Copy link
Owner

Hmm - that'd be a singleton, which is generally not going to be a good thing (npm dupe issues, etc). I've used that pattern before with a similar EventEmitter, and that was actually one of the reasons I wrote this one - you can do this:

import mitt from 'mitt';
const { on, off, emit } = mitt();
export { on, off, emit };

That's probably the better option if you're looking to then import the singleton methods later on. You get to use an instantiable library (mitt) but then work with your own created singleton in your codebase.

Just my 2 cents :)

@tunnckoCore
Copy link
Collaborator Author

tunnckoCore commented Jan 16, 2017

Actually, I did it :) But is 212bytes (tests passing) ;/ All of my above snippets was wrong. Idea is different.

We need to not just return an object with the methods but assign that object to const ret, return that const and use that const in each method.

export default function mitt(all) {
	// Arrays of event handlers, keyed by type
	all = all || {};

	// Get or create a named handler list
	function list(type) {
		let t = type.toLowerCase();
		return all[t] || (all[t] = []);
	}

	const ret = {

		/** Register an event handler for the given type.
		 *	@param {String} type		Type of event to listen for, or `"*"` for all events
		 *	@param {Function} handler	Function to call in response to the given event
		 *	@memberof mitt
		 */
		on(type, handler) {
			list(type).push(handler);
			return ret;
		},

		/** Remove an event handler for the given type.
		 *	@param {String} type		Type of event to unregister `handler` from, or `"*"`
		 *	@param {Function} handler	Handler function to remove
		 *	@memberof mitt
		 */
		off(type, handler) {
			let e = list(type),
				i = e.indexOf(handler);
			if (~i) e.splice(i, 1);
			return ret;
		},

		/** Invoke all handlers for the given type.
		 *	If present, `"*"` handlers are invoked prior to type-matched handlers.
		 *	@param {String} type	The event type to invoke
		 *	@param {Any} [event]	An event object, passed to each handler
		 *	@memberof mitt
		 */
		emit(type, event) {
			list('*').concat(list(type)).forEach( f => { f(event); });
			return ret;
		}
	};

	return ret;
}

@tunnckoCore
Copy link
Collaborator Author

tunnckoCore commented Jan 16, 2017

It will fit if we use #8. Damn.. that's freaking crazy haha

@developit
Copy link
Owner

Gotta get that size down somehow 😉 208 is pretty good though, basically just paying for one return ret thanks to gzip.

@dotproto
Copy link
Contributor

Seems like this issue and #1 are incompatible.

@developit
Copy link
Owner

Oooh - good point. I totally did not think of that.

@tunnckoCore
Copy link
Collaborator Author

tunnckoCore commented Jan 17, 2017

Yea, absolutely. But I believe this lib is meant to be with kind of compat with most common and to be small. Or at least it is promoted as that. I not mean that #1 will be with bigger size, but it is other thing and can be done in separate more smaller lib. 2c

@sospedra
Copy link

sospedra commented Jan 18, 2017

Instead of returning unsuscribe method (#1) we can add the getUnsuscribe on the fluent API. So you can get it whenever you need to.

@dotproto
Copy link
Contributor

@sospedra I'm not sure how viable a new method would be as the 200 B gzipped limit is quite a challenge to work around. @tunnckoCore has been doing some heroic with in #19.

@developit
Copy link
Owner

adding getUnsubscribe() would require that a fluent API return new instances of the interface for every call. I don't think that is possible given the size limitations here, and the performance would be quite poor.

@shshaw
Copy link

shshaw commented Jan 23, 2017

Using Closure Compiler, you end up with 197 bytes after including the return in each function by it converting all let or const to var. If using let & const aren't a requirement, that's an easy way to get that gzip size down.

function mitt(b){function c(a){a=a.toLowerCase();return b[a]||(b[a]=[])}b=b||{};var d={on:function(a,e){c(a).push(e);return d},off:function(a,e){var b=c(a),f=b.indexOf(e);~f&&b.splice(f,1);return d},emit:function(a,b){c("*").concat(c(a)).forEach(function(a){a(b)});return d}};return d};

@developit
Copy link
Owner

The source gets transpiled by Buble so those let declarations actually are var - good to know we could save a few bytes going with closure though.

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

Successfully merging a pull request may close this issue.

5 participants