make decirc function public

[marbemac]

We find it quite useful to remove circular references without having to go through a full stringify and then re-parse.

[davidmarkclements]

If we expose it publically it also needs documentation and unit tests

We may also want to consider the API

[marbemac]

Makes sense, I can do that - in general is this a change that you'd be interested in? For the simple case of needing to decycle an existing JS object, small objects jump from 100k -> 600k ops in our benchmarks, and large objects jump even more, so this has been great for us (rather than doing a JSON.parse(safeStringify(obj))).

Do you already have thoughts on possible changes to the API for this function? I would throw decycle out as a function name that might make more sense. On the API side of things we could toss some of the optional params into an opts obj at the end of the function, something like:

function decycle(
  val: any,
  opts: {
    parent?: any;
    parentKey?: string;
    stack?: any[];
  } = {}
) {
  const { parent, parentKey, stack = [] } = opts;

  // ... implementation
}

[davidmarkclements]

we would need to benchmark any api changes but your suggestions could potentially be more ergonomic

Would you be able to speak a little more to your use case? Why do you need to remove circular references if you're not serialising?

@arbridge @BridgeAR any thoughts?

[davidmarkclements]

cc @BridgeAR (sorry got the handle wrong)

[marbemac]

Yup, we're just as interested in benchmarks and performance as you are :).

Without getting into too much detail our primary use case is:

1. We have a resolver that recurses through objects. Simplistically, it replaces $ref string properties in the object with the values that the $ref points point to.
2. Step 1 can lead to circular references in the resulting JS object - it is not possible to protect against this for all edge cases without huge sacrifices in performance.
3. We have a lot of components and other utilities that recurse through the results of 2 to render it in our UI amongst other things. These older components and utilities are not all protected against circular refs in JS.

Let me know if it doesn't make sense or you need more detail.

[davidmarkclements]

Does this mean that there are potential cases where decirc would be used but stringify wouldn't?

If that is true, decirc should be a separate module - and fast safe stringify can rely on it

[BridgeAR]

I agree that moving the decirc function to a separate module will be best in this case. It will definitely not hurt one way or the other and if there are people who need such a functionality, it can be used that way.

@davidmarkclements are you going to open such a module or should I do that?

[davidmarkclements]

If you've got time go ahead - otherwise I can - either way let's both be contribs

[mcollina]

Definitely +1 in splitting. Add me as well (also on npm)!

[marbemac]

Ok after a bit of investigation, the original interface I proposed is definitely not the way to go. Passing around objects as function arguments decreases performance significantly (see below).

decycle (decirc) original

decycle:   simple object x 7,177,379 ops/sec ±1.60% (84 runs sampled)
decycle:   circular      x 2,718,914 ops/sec ±1.08% (91 runs sampled)
decycle:   deep          x 147,167 ops/sec ±1.36% (84 runs sampled)
decycle:   deep circular x 142,069 ops/sec ±1.15% (86 runs sampled)

decycle (decirc) with obj argument

decycle:   simple object x 4,906,651 ops/sec ±0.94% (88 runs sampled)
decycle:   circular      x 2,174,687 ops/sec ±1.33% (93 runs sampled)
decycle:   deep          x 134,019 ops/sec ±1.45% (86 runs sampled)
decycle:   deep circular x 133,005 ops/sec ±1.02% (84 runs sampled)

Since we're actively working with this stuff I'm continuing to make changes, feel free to use any or none of it. I'm also happy to help contribute if you guys want.

I went ahead and added an option for a custom replacer, and added the key to the stack. This allows easy tracking of where the original circular reference was pointing to, by using a simple custom replacer. You can find an example of how we use this to replace with a JSON pointer instead of '[Circular]' in the tests file.

The performance impact of the custom replacer is pretty negligible:

original

fast-safe-stringify:   simple object x 1,064,853 ops/sec ±0.97% (89 runs sampled)
fast-safe-stringify:   circular      x 560,530 ops/sec ±0.94% (92 runs sampled)
fast-safe-stringify:   deep          x 29,688 ops/sec ±1.29% (86 runs sampled)
fast-safe-stringify:   deep circular x 29,335 ops/sec ±1.23% (88 runs sampled)

with replacer

fast-safe-stringify:   simple object x 1,042,125 ops/sec ±1.05% (89 runs sampled)
fast-safe-stringify:   circular      x 545,805 ops/sec ±0.95% (91 runs sampled)
fast-safe-stringify:   deep          x 30,062 ops/sec ±1.05% (87 runs sampled)
fast-safe-stringify:   deep circular x 29,596 ops/sec ±1.06% (87 runs sampled)