stop polluting EventEmitter #200

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
6 participants
Contributor

JerrySievert commented Mar 9, 2012

vows pollutes the global EventEmitter when it wraps emit().

this causes issues in tests of code that also wraps emit() (such as EventReactor).

this patch simply replaces use of the global EventEmitter with EventEmitter2 -- problem solved.

Fix all the bugs!

Contributor

JerrySievert commented Mar 19, 2012

ping

pong

Contributor

JerrySievert commented Apr 2, 2012

adding @indexzero and @Marak so i don't have to shout across the fire pit.

Marak commented Apr 2, 2012

+1

zdzolton commented Jun 6, 2012

Keep the fire!

Contributor

cloudhead commented Jul 31, 2012

I don't think introducing another dependency is the solution to any problem - where exactly is the global object polluted?

Contributor

JerrySievert commented Jul 31, 2012

https://github.com/cloudhead/vows/blob/master/lib/vows.js#L223-237

this does not actually preserve the old emitter, and wreaks havoc when trying to test code that uses other packages that monkey punch EventEmitter (such as EventReactor).

Contributor

cloudhead commented Aug 7, 2012

Ah I see, wasn't aware of that patch.

Still, is there another way to - either implement the functionality differently, or fix it without introducing another dep?

Or by making the ordering optional.

Contributor

JerrySievert commented Aug 10, 2012

i will continue to look into it, leaving this open as a placeholder for now, in case someone else wants to chime in.

Owner

indexzero commented Nov 4, 2014

@JerrySievert with a fresh set of eyes this is actually "by design". Your changes to the test/* code show that this is a very breaking change.

vows is designed to trap unhandled error events and bubble them up to a reporter instead of just crashing the test process. You can see my analysis in the gh-200 branch

If this is something we want to do in a 1.0.0 that's OK with me, but it seriously changes the semantics of vows and would end up in there if we decide it's the best thing.

indexzero closed this Nov 4, 2014

indexzero added the v1.0.0 label Nov 4, 2014

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