scottcorgan / tiny-emitter

A tiny (less than 1k) event emitter library
MIT License
935 stars 67 forks source link

Annoying to use with ES6 imports and Browserify #16

Closed Dan503 closed 7 years ago

Dan503 commented 7 years ago

The expected way to import modules with ES6 and browserify is this:

import emitter from 'tiny-emitter';

emitter.on('event', ()=>{ /* functionality */ });

However there is an extra step involved in order to import tiny emitter into a project in a way that lets modules talk to one another across different files due to tiny-emitter exporting an uncalled javascript class.

//local emitter.js file

import Emitter from 'tiny-emitter';

export default new Emitter();
// module js file

import emiter from './path/to/emitter.js';

emitter.on('event', ()=>{ /* functionality*/ });

As you can see it feels like an unnecessary extra step and a tiny extra file in the file system.

What I would propose is having tiny emitter export an already called emitter rather than an uncalled class.

So instead of this as the export command (which is what you pretty much have at the moment):

export default Emitter;

You would have this:

const emitter = new Emitter();
export default emitter;
export { emitter, Emitter };

This would be a breaking change but I think it would be worth it for the ease of use it brings to your users.

The second export allows people to still alter/extend the class itself if they wish. The vast majority of the time, you only want to import the already called emitter though and this set up allows for that more easily.

I would have done the change myself in a pull request but I don't know typscript well enough to make the necessary edits.

Dan503 commented 7 years ago

Actually I just realised that the second export I suggested is completely pointless since you can still use emitter.prototype to make alterations to the emitter itself.

In that case all you need to export is this:

export default new Emitter():
scottcorgan commented 7 years ago

I'd like to keep a class as the default import, but I'm open to exporting a singleton as well.

import emitter from 'tiny-emitter/instance'

Or something similar.

How does that sound?

Dan503 commented 7 years ago

I'd prefer the instance to be the default but considering how popular this package is and that changing the default to an already called instance would be a breaking change I think that's a good compromise. That method would still achieve the desired result of a 1 line import without needing to introduce a breaking change.

So yeah, I'd be happy with that as the solution :)

Dan503 commented 7 years ago

An alternative solution would be this:

import { emitter } from 'tiny-emitter';

People can customise the variable by doing this:

import { emitter as customEmitterName } from 'tiny-emitter';

That is probably the more correct solution but I don't mind which ever one you go with.

scottcorgan commented 7 years ago

I did want to avoid instantiating an used object. That was why I suggested another file to import.

I'll think about it a bit more. Thanks for the idea!.

scottcorgan commented 7 years ago

Closed with https://github.com/scottcorgan/tiny-emitter/pull/18