Introduce trace flag to mark circular reference keys (#1) #3
Conversation
|
Can you add a test where the trace refers to a nested property? |
| //=> TypeError: Converting circular structure to JSON | ||
|
|
||
| console.log(safeStringify(foo, {trace: true})); | ||
| //=> '{ "a": true, "b": "[Circular *]" }' |
There was a problem hiding this comment.
I think the example should be moved to the trace option docs.
| //=> TypeError: Converting circular structure to JSON | ||
|
|
||
| console.log(safeStringify(foo, {trace: true})); | ||
| //=> '{ "a": true, "b": "[Circular *]" }' |
There was a problem hiding this comment.
| //=> '{ "a": true, "b": "[Circular *]" }' | |
| //=> '{"a":true,"b":"[Circular *]"}' |
There was a problem hiding this comment.
It would be better to use an example that shows a named pointer *c.
|
|
||
| ##### trace | ||
|
|
||
| Type: `boolean` |
There was a problem hiding this comment.
| Type: `boolean` | |
| Type: `boolean`\ | |
| Default: `false` |
| /** | ||
| Enable key tracing for circular references. | ||
| By default, this option is set to false. |
There was a problem hiding this comment.
Use the default notation: https://github.com/sindresorhus/typescript-definition-style-guide#documentation
| readonly indentation?: string | number; | ||
|
|
||
| /** | ||
| Enable key tracing for circular references. |
|
|
||
| Type: `boolean` | ||
|
|
||
| Enable key tracing for circular references. |
There was a problem hiding this comment.
I think the description should be clearer about what it does.
|
@GeorgeGkas Still interested in finishing this? :) |
|
Hey @sindresorhus. I'm looking into this a bit more and I believe we still have not covered all the cases. The current implementation has a bug. Consider the following example: const foo = {a: true};
foo.b = {};
foo.c = foo.b;If we run {
"a": true,
"b": {},
"c": "[Circular *b]"
}where the correct output should be: {
"a": true,
"b": {},
"c": {}
}Even if there is no circular dependency, the algorithm thinks otherwise. This results from the way we check for circular dependencies. Unfortunately, I don't have time to look into this in more depth now. Should I close this PR and come back again with a better |
|
I'm aware of that issue and intend to look into it, but I think that issue needs to block this pull request. |
|
Closing for lack of activity. |
2 participants
Fixes #1