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

Make handling circular references configurable #74

Closed
whitlockjc opened this issue Feb 5, 2016 · 9 comments
Closed

Make handling circular references configurable #74

whitlockjc opened this issue Feb 5, 2016 · 9 comments

Comments

@whitlockjc
Copy link
Owner

We need to make it where a configuration option can dictate whether circular references are replaced with its original definition or {}.

@brettz9
Copy link
Contributor

brettz9 commented Feb 9, 2016

Would it be equally difficult to support resolving into genuinely cyclic (non-JSON) objects as with your prior support of depth?

(It appears my validator (Ajv) can support circular schemas, so building cyclic objects might work out nicely in the absence of depth (or even preferable to it in some cases, by its avoiding memory problems and uncertainty as far as the depth one would require).)

@whitlockjc
Copy link
Owner Author

Well, if your validator can resolve relative references it should also be able to resolve the circular reference for validation. ;) In all seriousness, I could end up with bringing back depth support but I do remember it being a pain...although with the new API it could be trivial.

@cen1
Copy link

cen1 commented Mar 6, 2017

Any ETA on this? It breaks a major tool (swagger-ui) and has been open for over a year.

@whitlockjc
Copy link
Owner Author

whitlockjc commented Mar 6, 2017

How is swagger-ui impacted? They use swagger-js which has its own resolver and does not use json-refs. PRs welcome.

@cen1
Copy link

cen1 commented Mar 6, 2017

This issue was referenced in swagger-api/swagger-editor#1005 perhaps incorrectly.

@whitlockjc
Copy link
Owner Author

Ah, swagger-editor. That's different than swagger-ui. I'm currently rewriting the resolver and once it's done, this will be high on the list. I just don't have as much time on these projects these days since I don't do them as part of my day job. I'm working on it.

@whitlockjc
Copy link
Owner Author

What if json-refs, and thus sway and all tools using them, stopped trying to circumvent (remove/replace) circulars and instead resolved references as-is?

@whitlockjc
Copy link
Owner Author

The reason circulars were originally removed/replaced was due to tooling I was building around not supporting circular documents. This wasn't fair. I do think I could make it configurable but my worry is that no one will ever be completely satisfied.

@whitlockjc
Copy link
Owner Author

options.allowCircular was added. If you set it to true, circular references will be resolved allowing the caller to do whatever they want with the circular documents. There is no good way to add depth or another more configurable option without completely decimating performance so I think this is a good middle ground until we can come up with something better, if people still want something else now that we have this new feature.

whitlockjc added a commit that referenced this issue Apr 23, 2017
`options.allowCircular` was added to the resolution API options.  This
will allow callers to allow circular references to be resolved instead of
leaving them unresolved.

Fixes #74
whitlockjc added a commit that referenced this issue Apr 23, 2017
`options.resolveCirculars` was added to the resolution API options.  This
will allow callers to allow circular references to be resolved instead of
leaving them unresolved.

Fixes #74
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

No branches or pull requests

3 participants