2 ms·
The EventEmitter pattern seems interesting! I can't say I've wrapped my head around it enough to weigh the benefits and drawbacks. I would point out a few thing
by brianshaler 5y ago
The EventEmitter pattern seems interesting! I can't say I've wrapped my head around it enough to weigh the benefits and drawbacks. I would point out a few things after a quick look:
* It looks like you're creating an emitter wrapper around an EventEmitter instance, freezing it, then exporting the instance rather than the frozen wrapper[0]. I see .on() and .off() are wrapped here, while the project appears to use .addListener() and .removeListener() instead.
* As a typescript project, I would recommend using typed-emitter[1]. Not only does it ensure you're only emitting recognized events, it ensures that the type of the payload is correct for the corresponding event. Currently, your typed handlers are coercing from `any` as far as your IDE is concerned, rendering it unable to help prevent you from mistakes.
* Instead of plumbing an emitter prop everywhere, even through components that don't interact with it directly, this looks like a better fit for the Context pattern to create the global emitter instance in a top-level context provider, then getting a reference to the emitter via useContext[2].
[0] https://github.com/bootrino/reactoxide/blob/master/reactoxide/src/emitter.js https://github.com/bootrino/reactoxide/blob/master/reactoxid...
[1] https://www.npmjs.com/package/typed-emitter https://www.npmjs.com/package/typed-emitter
[2] https://reactjs.org/docs/context.html https://reactjs.org/docs/context.html
- andrewstuart 5y agoGood advice thanks!