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

fix example rendering #198

Merged
merged 1 commit into from
Feb 26, 2020
Merged

Conversation

Damokl3s
Copy link

This fixes an issue we were seeing where large schemas would have missing properties in the examples.

Previously, because options are shared between recursive calls, only a maximum of 100 properties would be rendered in total. (It would be less if $ref are used, because the indirection counts towards the limit).

Since the fix in #190 prevents circular examples from ending in an infinite loop, I removed the "depth" check. I've tested it with the referenced billing.yaml example and it works fine. If you prefer I can leave it in, though since it now truly checks for depth, 100 would be a pretty high limit, so I don't think it adds anything.

Also if the same $ref is used multiple times in the same schema, even if not recursive, it would previously only render the first occurrence.

Here's an example of billing.yaml before and after:
Bildschirmfoto 2020-01-30 um 09 12 11

Notice that id was missing in a lot of places because it is a $ref and is used on the top level already, so it was not render it in nested objects.

Recursive examples like this still work: (Service object which contains a nested Service object ServicePeriod which has nested ServicePeriod)
Bildschirmfoto 2020-01-30 um 09 13 04

@Damokl3s Damokl3s requested a review from auscaster February 24, 2020 14:43
@auscaster
Copy link
Member

Looks good, sorry for the wait! Merging now :)

@auscaster auscaster merged commit dfca985 into sourcey:master Feb 26, 2020
@Damokl3s
Copy link
Author

Thanks! No worries.

Would be awesome if you could push an updated version to npm :)

@auscaster
Copy link
Member

Just pushed 1.1.0

@Damokl3s
Copy link
Author

Awesome, thank you very much!

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.

2 participants