Open dillonredding opened 4 years ago
This is intended behavior, and is documented here: https://github.com/whitlockjc/json-refs/tree/master/docs#resolution Long story short, circular references are a problem for many tools and so we will leave circular references as-is in the provided document. There is some grey area here, as documented. If this explains things, please feel free to close the issue.
I think this behavior is fine for local circular references. The issue is when there's a local circular reference in an externally referenced document. As seen in my example, the reference from the external document is copied over and now references nothing. I would think the value of the $ref
could be updated to point to the where it is in the file being resolved.
Reading back over the example I gave, I preemptively resolved the reference in foo.yaml
. That should be:
Foo:
type: object
properties:
bar:
$ref: bar.yaml#/Bar
That's how it should work. Maybe we're not doing that and this would in fact be a bug. I'll get this fixed ASAP!
Just curious, is there an ETA on this?
I'll get it out today. Sorry, priorities didn't let me get to it sooner.
I have this fixed locally, but I need to upgrade gulp so that TravisCI passes. My plan is to update all dependencies, and gulp, commit and once the build is passing, I'll cut the release.
Good news is that while working on this, I found two other circular-reference resolution bugs and fixed them too.
I just went ahead and cut a release. json-refs@3.0.14
fixes this issue.
@dillonredding Do you mind verifying this implemented what you were requesting?
Not quite. Here's what I get:
foo.yaml:
Foo:
type: object
properties:
bar:
$ref: 'bar.yaml#/Bar'
bar.yaml:
Bar:
type: object
properties:
foo:
$ref: '#/Bar'
Command:
json-refs resolve foo.yaml
Expected Output:
Foo:
type: object
properties:
bar:
type: object
properties:
foo:
$ref: '#/Foo/properties/bar'
Actual Output:
Foo:
type: object
properties:
bar:
type: object
properties:
foo:
$ref: '\bar.yaml#/Bar'
From a consistency perspective, what was implemented makes sense. The idea is that when you have a circular reference, we leave it as-is and let whatever tooling is there to resolve it. The bug you reported is that for locally-circular references in nested documents, doing that (keeping the local reference as-is) meant that the resolved value was left in a state where it's unresolvable in the resolved document. That problem is fixed now, as locally-circular references in nested documents are fully-qualified when injected into the resolved document, allowing the resolved document to reference the documents where the circular chain was introduced.
That being said, I almost took a path similar to what you are wanting back in the day and was worried it was too much magic. Manipulating $ref
values wasn't something I wanted to do...I either wanted to replace/resolve them or leave them as-is. But I guess now, this new approach, does in fact manipulate $ref
values by making them full-qualified in the aforementioned case. (Good news though, the reference metadata retains the original definition as expected.)
I would definitely say that in one way, maybe not exactly as you wanted, the original description of the issue is fixed. But let me give this some thought, to see if something jumps out at me in the circular processing that is making this behave different than intended.
Yeah, what I'm asking for might be pretty specific. Specifically, I'm wanting to remove the references outside the file and have it be completely self-contained. I have no issues with manipulating $ref
s as long as the file remains "equivalent", but I might not be considering every scenario.
I get what you mean, and it might be easier than I thought. I'm going to reopen this to avoid it falling through the cracks.
This might already be a known issue, but I haven't found anything exact. There seems to be an issue resolving external references that are recursive. Here's an example (a bit contrived, but narrowed down to the essentials):
foo.yaml:
bar.yaml:
But when resolving through the CLI with
json-refs resolve foo.yaml
, I get a non-existent reference:This might be expected behavior (I'm not well versed in the JSON Reference specification); however, one workaround that comes to mind would be to update the reference to where it's actually resolved:
Version of
json-refs
: 3.0.13