Skip to content

Conversation

pangoyal
Copy link
Contributor

PR in regard to #19

@pangoyal pangoyal mentioned this pull request Aug 16, 2017
@mohsen1
Copy link
Owner

mohsen1 commented Aug 16, 2017

I usually recommend resolving refs before passing it to this directive.
Automatically rendering refs might not be desired. Please add an option for enabling this and make it off by default.

@pangoyal
Copy link
Contributor Author

Can do that although IMHO refs are integral part of jsonschema so whats the harm in displaying them?

@mohsen1
Copy link
Owner

mohsen1 commented Aug 17, 2017

Per JSON Schema spec, refs are not part of json schema. I appreciate your work and am interested in merging this.

@pangoyal
Copy link
Contributor Author

pangoyal commented Aug 17, 2017

What you talking about. Its there in the spec. http://json-schema.org/latest/json-schema-core.html#rfc.section.8

They are heavily used too (atleast in the Swagger specs). We can make it an optional feature to show them. That works for my needs but I personally think they should be handled out of the box.

@pangoyal
Copy link
Contributor Author

@mohsen1 still not convinced about having it handle refs by default? I will make it optional if thats what you want.

@pangoyal
Copy link
Contributor Author

pangoyal commented Sep 8, 2017

Okay. Made it optional. Pls check and release it. My product depend on this

const el = view.render();
const code = el.querySelector('.inner.oneOf').innerHTML;
expect(code.indexOf('<a class="title">Ref </a>')).not.to.equal(-1);
expect(code.indexOf('<a class="type" href="http://foo.bar/additionalBloodTypes">http://foo.bar/additionalBloodTypes</a>')).not.to.equal(-1);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this is passing without passing showRef option?

@mohsen1
Copy link
Owner

mohsen1 commented Sep 8, 2017

+1 I'll merge once the test is fixed.

@valasatava
Copy link

Handling $ref's will be very useful. Is there an intent to proceed with this PR ay time soon?

@mohsen1
Copy link
Owner

mohsen1 commented Aug 17, 2018

I don't have time maintaining this. My suggestion is to resolve refs using json-refs npm package before sending it over to this package.

@valasatava
Copy link

@mohsen1, thank you for suggestion!

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.

3 participants