Skip to content

Conversation

@nlundquist
Copy link

@nlundquist nlundquist commented Jun 11, 2020

Hi, I wanted to share my early work towards an issue I've seen reported in #72 and others, and in the issues sections of tools consuming json-schema-ref-parser.

Essentially, there's a desire among users to change the shape of the root schema so that when bundling occurs, the resulting schema has dereferenced references into new "shallow" positions in the root, e.g a reference deep in a schema to foo.bar#/definitions/baz is dereferenced into #/definitions/baz, even though no such reference under #/defintions existed in the root schema.

Of course, this sort of functionality can be had by manually adding shallow references like "#/definitions/baz" to the root schema, but in sufficiently complex cases this is too great a burden to ask of authors.

What this PR does is automatically add shallow references to the root schema during bundling, resulting in a schema that can be understood by https://github.com/OpenAPITools/openapi-generator and is in general easier for humans to read due to the "flatter" structure.

If you feel that this functionality lies outside the scope of this project, would you consider moving the sort function as I've done and adding a pre-remap hook so that this feature could be implemented by users how they desire at runtime?

Thanks for all your hard work maintainers 🥇

@nlundquist nlundquist changed the title WIP: add "shallow reference" bundling functionality add "shallow reference" bundling functionality Jun 15, 2020
@marbemac
Copy link
Contributor

Hi @nlundquist, while I'm not an official maintainer on this project, IMHO this seems like an interesting addition! I've thought about this in the past, and there are some tricky bits.

One question - how are you thinking about local pointer collision?

For example - a couple of slightly different use cases:

{ "responses": { "200": { "$ref": "./successful.json#/definitions/response" }, "500": { "$ref": "./failure.json#/definitions/response" } } }

or:

{ "definitions": { "user": { "allOf": [ { "$ref": "./models.json#/definitions/user" }, { "type": "object", "properties": { "name": { "type": "string" } } } ] }, } }

The above could make for some good test cases :).

@nlundquist
Copy link
Author

nlundquist commented Jun 16, 2020

Thanks for the interest in this :)

Currently, this code simply throws an error when it detects "path collisions". For my personal use case that's all that I needed.

If this was adopted into the project I'd support an optional argument that would be a "shallow position" resolving function, allowing users to do any renaming / special path changes they might need.

I don't think there's a perfect way to handle collisions so I'd personally opt for a simple implementation, with an error/warning explaining the limitations and leave it for users to decide how they want to avoid them. My first suggestion to users would always be to consider changing paths to prevent collisions before going down the road of renaming / repathing.

@jrnail23
Copy link

@nlundquist, if I understand your intent correctly, this looks like it would also be quite valuable when converting JSON schemas to Swagger / OpenAPI docs, since OpenAPI seems to only support refs to definitions defined at the root level.
Incidentally, this just happens to be my current use case!

@lechiefe
Copy link

Hi, wanted to check if there is any intention to merge this change in? We really need this feature. Thanks

@nlundquist
Copy link
Author

nlundquist commented Dec 18, 2020

I'm still interested in getting this merged and am willing to make any adjustments the maintainers require.

If they would rather not support a non-standard feature like this, I'd only ask that they provide a pre-remap hook into the bundling process. A callback that can modify the parsed collection of refs is all that's needed. With that I could release my implementation as it's own dependency.

I've been using my fork in a production application since I created it. I wont delete it even after this PR is closed so feel free to depend on that until this feature lands.

@nlundquist
Copy link
Author

@JamesMessinger could you weigh in on if you'd like to see a feature like this end up in the project?

@philsturgeon
Copy link
Member

philsturgeon commented Dec 21, 2020 via email

@manuelottlik
Copy link

Hey, are there any updates on this? Would love to see an option to use shallow references!

@nlundquist
Copy link
Author

I'm guessing that @philsturgeon would rather not merge this PR given his comment here: #72 (comment)

Phil mentions that Stoplight has their own internal tool that satisfies their need for this sort of opinionated rewriting. I may investigate providing such a tool considering the public need for one.

@philsturgeon
Copy link
Member

Phil mentions that Stoplight has their own internal tool that satisfies their need for this sort of opinionated rewriting.

That, but also the lack of tests make it hard to see what i actually going on and weigh it up on its own merit.

If you ask 100 people how to bundle I think you'll get 100 answers, but a tool which extends JSON Schema Ref Parser and helps you come up with custom solutions could be pretty excellent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants