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

fixes issue where multiple requires causes multiple writes #18

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@timsavery

timsavery commented Jul 25, 2013

I came across an issue while writing a CLI app where require('charm')(process.stdout) being called once inside of app.js and once inside of some_module.js causes duplicate output. This effect is additive for each time the module is required with the same stream.

This pull request fixes this problem by changing Charm.prototype = new Stream; to Charm.prototype = Stream.prototype.

Here is a snippet that reproduces the described issue:

var charm1 = require('../index')(process.stdout);

charm1.write('from charm1\n\n');

var charm2 = require('../index')(process.stdout);

charm1.write('from charm1\n');
charm2.write('from charm2\n');

You will notice that before charm is required a second time the text is only printed once, after the second time each message is duplicated.

@papandreou

This comment has been minimized.

papandreou commented Dec 2, 2013

+1. In node.js 0.10 the EventEmitter constructor explicitly initializes this._events, so going Charm.prototype = new Stream; causes all Charm instances to share the same _events property, which is extremely bad.

@substack, please merge.

papandreou added a commit to papandreou/node-charm that referenced this pull request Dec 2, 2013

@papandreou

This comment has been minimized.

papandreou commented Jan 29, 2014

@substack Ping? This breaks difflet and 40+ other dependent modules under node.js 0.10+.

@ahmadassaf

This comment has been minimized.

ahmadassaf commented Oct 7, 2014

+1. This also fixes issues when using multiple modules that uses Charm. I am using pace to show multiple progress bars and they break showing multiple writes
screen shot 2014-10-07 at 19 01 56

@sompylasar

This comment has been minimized.

sompylasar commented Nov 4, 2014

@timsavery Sharing a reference to the prototype object Charm.prototype = Stream.prototype will lead to propagation of every change in the Charm.prototype to the Stream.prototype.

node has got the inherits function in the util module to implement inheritance.
http://nodejs.org/docs/latest/api/util.html#util_util_inherits_constructor_superconstructor

@cryptoquick

This comment has been minimized.

cryptoquick commented Feb 10, 2015

So, I'm still seeing this problem in one of the modules I'm using, and I'm wondering when this might be resolved.

@martinheidegger

This comment has been minimized.

martinheidegger commented Jun 23, 2015

@timsavery This PR should be closed in favour of #24

@timsavery

This comment has been minimized.

timsavery commented Jun 24, 2015

Closing pull request in favor of #24

@timsavery timsavery closed this Jun 24, 2015

@timsavery

This comment has been minimized.

timsavery commented Jun 24, 2015

@Samreay Samreay referenced this pull request Feb 14, 2016

Open

Inherit Stream properly #24

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