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

Handle circular references #84

Merged
merged 9 commits into from
Feb 8, 2019
Merged

Handle circular references #84

merged 9 commits into from
Feb 8, 2019

Conversation

eemeli
Copy link
Owner

@eemeli eemeli commented Feb 5, 2019

Fixes #81

Essentially, this now works:

const a = ['b']
a.push(a)
const str = YAML.stringify(a)
// &a1
// - b
// - *a1
YAML.parse(str)
// [ 'b', [Circular] ]

For stringifying, this means that input that would earlier throw now doesn't, producing valid YAML instead. When parsing, alias nodes referring to anchors now resolve the original objects, rather than separate instances. I'm not sure yet whether this should mean a major version bump or not.

The public YAML.createNode() and the doc.schema.createNode() APIs have not changed, and do not support the autogeneration of alias nodes.

@isaacs
Copy link
Contributor

isaacs commented Feb 5, 2019

Fantastic! I'll play with this tonight and provide some feedback. Thanks!

@isaacs
Copy link
Contributor

isaacs commented Feb 8, 2019

As far as I can tell, this works perfectly.

My only complaint is that there doesn't appear to be a way to disable the warning from being printed to stderr.

// dist/Document.js:455
if (!src.resolved) {
  const msg = 'Alias node contains a circular reference, which cannot be resolved as JSON';
  this.warnings.push(new _errors.YAMLWarning(node, msg));
}

// dist/index.js:66
doc.warnings.forEach(warning => console.warn(warning));

If it handles circular references fine, then so what? If I'm using YAML to stringify and parse, who cares that it doesn't make for valid JSON?

@eemeli
Copy link
Owner Author

eemeli commented Feb 8, 2019

You're right, that warning doesn't make sense any more, so I removed it.

@eemeli eemeli merged commit 72674ea into master Feb 8, 2019
@eemeli eemeli deleted the cycles branch February 8, 2019 23:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support circular objects (using Anchors?)
2 participants