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

add stringify helper #3

Open
tkellen opened this issue Nov 7, 2012 · 6 comments
Open

add stringify helper #3

tkellen opened this issue Nov 7, 2012 · 6 comments

Comments

@tkellen
Copy link
Member

tkellen commented Nov 7, 2012

via @jsoverson on IRC

It works, which is good. EventEmitter2 has what looks like some issues unregistering listeners, and communication involving cyclic structures breaks when jsonifying data. I had to implement a custom stringify method that might be beneficial to incorporate into the lib.

@tkellen
Copy link
Member Author

tkellen commented Nov 7, 2012

@jsoverson Could you expand on this a little bit? Ben and I are both unclear about what is needed here. Thanks!

@jsoverson
Copy link
Member

In both contrib-jasmine and contrib-qunit there has to be a sendMessage function that JSON stringifies its message/data and then issues an alert in order to communicate with phantomJS.

Standard JSON stringification doesn't deal well with cyclic structures so, when the input may be unknown (like when reporting expected vs actual in unit tests) the bridge may break if it gets a complex structure with circular references.

sendMessage in qunit : https://github.com/gruntjs/grunt-contrib-qunit/blob/master/phantomjs/bridge.js#L19
sendMessage in jasmine : https://github.com/gruntjs/grunt-contrib-jasmine/blob/master/tasks/jasmine/reporters/PhantomReporter.js#L9

The stringify method I had to write to protect against it: https://github.com/gruntjs/grunt-contrib-jasmine/blob/master/tasks/jasmine/reporters/PhantomReporter.js#L97

The sendMessage method should probably exist in grunt-lib-phantomjs along with whatever stringify protection is necessary.

I ended up being concerned about performance so I only used the stringify method where I thought it would be most necessary, but I don't think performance cost ended up being significant and it could be rolled into sendMessage itself.

@vladikoff
Copy link
Member

It's been a year with this issue, what are you guys thinking regarding this stringify helper?

@jsoverson
Copy link
Member

@sindresorhus had an object stringification method used in yeoman. It should probably still be added, but his might have gone under more revisions.

@sindresorhus
Copy link
Member

@jsoverson stringify-object is for printing out formatted JS objects, not for messaging. You probably want https://npmjs.org/package/json-stringify-safe

@Arkni
Copy link
Member

Arkni commented Feb 13, 2016

It's been ~3 years with this issue. Are you going to implement the helper or using the module that sindersorhus points to or something else ?

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

No branches or pull requests

5 participants