Support circular objects (using Anchors?)

[isaacs]

Consider this js:

const yaml = require('yaml')
const a = {}
a.a = a
console.log(yaml.stringify(a))

The result is a stack overflow, which is not very good. I'd really like to avoid having to work around this by sanitizing objects prior to sending to the yaml stringifier.

One solution would be to keep a list of all the objects visited earlier in a traversal, and print [Circular] or something.

Another would be to take advantage of yaml anchors, so the above object would stringify as:

---
&generated_anchor
a: *generated_anchor
...

or something?

You probably don't want to litter the output with anchors on every map and sequence, so maybe it should only do this if it sees that an object is repeated and walks up to the parent instance to add the anchor where needed.

[isaacs]

It looks like this approach is used in js-yaml:

> a = jsYaml.safeLoad('&ref\na: *ref')
{ a: [Circular] }
> jsYaml.safeDump(a)
'&ref_0\na: *ref_0\n'

[eemeli]

I really like this idea. The right place for the JS -> YAML circular reference detector is almost certainly during createNode, which probably needs to track seen objects.

For the YAML -> part of the loop, we already have a detector, but for now it's just failing rather than returning an object.

[eemeli]

This is looking like it'll need quite some refactoring to make work. At the moment it's effectively the schema that creates nodes, and the schema doesn't know about the document or its anchors. Trying to figure out how to fix this relatively cleanly without needing a major version bump.

The stringification is currently able to transparently handle documents that have had some of their inner contents manually set to non-node values. Supporting circular references among all such modified contents is looking like it'd require some rather hairy rewinding and re-stringification, so at least some features won't make sense to implement right away.

[isaacs]

You’re talking about cases like this?

a: *a
  b: 1
b: &a
  c: 2

I think this is the last piece that ideally would be in place to move all the tap modules to this yaml implementation, but I can temporarily keep the logic to walk the object and clobber circular refs before stringifying.

If it only worked for self-nested recursive objects, that’d be ok. That’s the case that causes the stack overflow. Ie, if a repeated object like a={};yaml.stringify([a,a]) returns [{},{}], then that’s fine.

[eemeli]

Sort of related, but not quite. I mean more what happens when you create a YAML.Document and while working with its contents end up assigning an object (possibly a node) to more than one place. Adding references between those while that document is then being stringified is tricky. So the point of my second paragraph was that at least to start with, doing it that way wouldn't work.

Accounting for references within a single treelike structure (as happens e.g. within any YAML.stringify() call) is relatively simpler, but I want to avoid having to break the existing API and having to do a major version bump because of this. I'll try and work on this over the next week or so, while travelling.

Btw, your sample case isn't valid YAML, as it's (1) referring to an anchor before it's defined, and (2) considering a value as both an alias node and a mapping. Presumably this is sort of what you intended?

a: &a
  b: 1
b:
  <<: *a
  c: 2

When that's parsed as YAML 1.1, it becomes:

{ a: { b: 1 }, b: { b: 1, c: 2 } }

As a side note, there probably is a way to model JS prototype chains using << merge keys in the YAML representation, but that's unlikely to be actually relevant to any real-world use case.