search

gruntjs / grunt-lib-phantomjs

Grunt and PhantomJS, sitting in a tree.
http://gruntjs.com/
MIT License
93 stars 63 forks source link

add stringify helper #3

Open tkellen opened 11 years ago

tkellen commented 11 years ago

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 commented 11 years ago

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

jsoverson commented 11 years ago

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 commented 10 years ago

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

jsoverson commented 10 years ago

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

sindresorhus commented 10 years ago

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

Arkni commented 8 years ago

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 ?