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

Use json-stringify-safe for json formatter #35

Closed
rooftopsparrow opened this issue Jan 7, 2017 · 4 comments · Fixed by #41
Closed

Use json-stringify-safe for json formatter #35

rooftopsparrow opened this issue Jan 7, 2017 · 4 comments · Fixed by #41

Comments

@rooftopsparrow
Copy link

rooftopsparrow commented Jan 7, 2017

Hello @TomFrost and contributors,

Wondering if you'd be open to taking on another dependency and changing the default json formatter to use json-stringify-safe to protect crashing the process when trying to stringify a circular structure?

I'm willing to make a PR if this is something you'd like.

Thanks 👍

@TomFrost
Copy link
Owner

TomFrost commented Jan 9, 2017

Another good suggestion!

My only immediate concern about this would be serialization speed. I'd love to see a benchmark of JSON.stringify versus that lib-- if you wanted to convert Bristol and test that, I wouldn't be opposed!

@rooftopsparrow
Copy link
Author

Awesome!

Since there are no benchmarking tests in Bristol right now, I'll make a PR adding some initial benchmarks to establish a baseline of performance. However, I have no clue what I'm doing in terms of benchmarking, so it'd be awesome to get some feedback on things you'd like to see benchmarked.

@MarkHerhold
Copy link
Contributor

@TomFrost json-stringify-safe is infinitely faster on circular references ;)

@yknx4
Copy link
Contributor

yknx4 commented May 29, 2017

PR #41

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

Successfully merging a pull request may close this issue.

4 participants