V.2

[BridgeAR]

Please have a look at the individual commits for a more detailed description.

[BridgeAR]

I rewrote the whole logic and added a completely new algorithm for the stable part that outperforms any to me known alternative.

[BridgeAR]

I am not sure how to enter the type information by the way - because it is a function including a property. So it is something in-between a interface and a declaration.

[davidmarkclements]

for curiosity/posterity, could you please also post comparison benchmarks (master vs pr branch)

[BridgeAR]

Old

fastSafeStringifyBench*10000: 37.160ms
fastSafeStringifyCircBench*10000: 55.967ms
fastSafeStringifyDeepBench*10000: 414.544ms
fastSafeStringifyDeepCircBench*10000: 440.958ms
fastSafeStringifyBench*10000: 32.556ms
fastSafeStringifyCircBench*10000: 45.782ms
fastSafeStringifyDeepBench*10000: 428.917ms
fastSafeStringifyDeepCircBench*10000: 426.085ms

New

fastSafeStringifyBench*10000: 32.890ms
fastSafeStringifyCircBench*10000: 39.071ms
fastSafeStringifyDeepBench*10000: 357.374ms
fastSafeStringifyDeepCircBench*10000: 370.444ms
fastSafeStringifyBench*10000: 25.070ms
fastSafeStringifyCircBench*10000: 36.617ms
fastSafeStringifyDeepBench*10000: 352.785ms
fastSafeStringifyDeepCircBench*10000: 358.732ms

[davidmarkclements]

👏

[mcollina]

Nice! I don’t understand the change exactly, what is the stable version?

pino-noir should be checked, it was relying on internals of this library to do its job.

[davidmarkclements]

ok @mcollina not knowing "stable version" definitely indicates this needs more explanation, and possibly a method rename

@mcollina see https://www.npmjs.com/package/json-stable-stringify

[davidmarkclements]

really good point re pino-noir - @BridgeAR there's a tight coupling between these two libraries, although from some output you showed me earlier it seems like you've already been working with pino-noir ?

[BridgeAR]

I updated the documentation a bit.

[davidmarkclements]

In terms of the d.ts file, I think this doc has details on how to implement for function-objects

https://www.typescriptlang.org/docs/handbook/declaration-files/templates/module-function-d-ts.html

[davidmarkclements]

@BridgeAR – great job clarifying in docs update

can we rename the stable method to stableStringify (or even aliasing is fine)

Two reasons:

1. it's clearer
2. allows for:

const { stableStringify } = require('fast-safe-stringify')

[BridgeAR]

Hmm... this will indeed break pino-noir... :cry: I will have to investigate how to circumvent that... The issue is that it is expected that pino-noir is able to manipulate the object that is passed to JSON.stringify. It will try to add properties to the circular object. And here it breaks because the way this code works is to replace the value with the string directly and replace it back to the original object before finishing...

Besides that: I am fine with renaming the function name.

[BridgeAR]

I do not yet know how to solve this.

[mcollina]

This is semver-major anyway, we would have to so a semver-major in pino and pino-noir too. It might take a bit more time to roll this out in pino.

[davidmarkclements]

the main thing we want to avoid is slowing pino-noir down

worst case scenario, we keep pino-noir on version 1 - it doesn't need the additional features

[BridgeAR]

The stable version passes all tests as it circumvents the problem (with a way that I am not completely happy about and it contains a TODO...). Doing something similar lets the pino tests pass but one stringify test fails and fixing that would be performance penalty again.

I am going to look into it. I might also have a look at pino if you like me to do that.

[davidmarkclements]

absolutely!

[BridgeAR]

I added a few more commits and highlighted the differences to JSON.stringify better.

I also added a alias and I made the tests a bit nicer.

When looking in how I can fix the breaking behavior for pino-noir I found that it would be possible to use a object with toJSON that returns the necessary string. That makes almost all tests pass (since it is a object again, it is possible to add properties on it again) in pino-noir but not all. I tried to play around with that a bit further but since it would not fix everything I decided against changing that.

It is just not possible to manipulate the input value while using toJSON or the replacer.

[mcollina]

Can you add a .travis.yml?

[BridgeAR]

With the new benchmarks it is clear that this will only improve the performance for simple objects. If the objects gets more complicated it will not work anymore.

[BridgeAR]

Ignore my last comment. Seems like a background task was running and ruined the results... you could of course also run the benchmarks again on your side.

util.inspect:          simple object x 276,599 ops/sec ±0.58% (96 runs sampled)
util.inspect:          circular      x 119,370 ops/sec ±0.62% (93 runs sampled)
util.inspect:          deep          x 18,910 ops/sec ±1.70% (90 runs sampled)
util.inspect:          deep circular x 18,602 ops/sec ±1.02% (94 runs sampled)

json-stringify-safe:   simple object x 231,850 ops/sec ±0.72% (94 runs sampled)
json-stringify-safe:   circular      x 110,717 ops/sec ±0.76% (90 runs sampled)
json-stringify-safe:   deep          x 8,634 ops/sec ±0.66% (96 runs sampled)
json-stringify-safe:   deep circular x 8,427 ops/sec ±0.71% (96 runs sampled)

fast-safe-stringify:   simple object x 1,089,387 ops/sec ±0.70% (95 runs sampled)
fast-safe-stringify:   circular      x 565,820 ops/sec ±0.31% (97 runs sampled)
fast-safe-stringify:   deep          x 31,922 ops/sec ±1.12% (92 runs sampled)
fast-safe-stringify:   deep circular x 31,804 ops/sec ±0.59% (95 runs sampled)

legacy:                simple object x 709,765 ops/sec ±0.85% (95 runs sampled)
legacy:                circular      x 359,440 ops/sec ±0.37% (98 runs sampled)
legacy:                deep          x 26,412 ops/sec ±2.67% (91 runs sampled)
legacy:                deep circular x 26,415 ops/sec ±0.71% (94 runs sampled)

[BridgeAR]

This should be ready. @mcollina @davidmarkclements would you be so kind and have another look?

[BridgeAR]

Rebased

[davidmarkclements]

great work @BridgeAR – published v2.0.0

now let's see what we can do with pino-noir