search

fuhrysteve / marshmallow-jsonschema

JSON Schema Draft v7 (http://json-schema.org/) formatting with marshmallow
MIT License
209 stars 72 forks source link

Supporting recursive nested schemas #36

Closed stringfellow closed 7 years ago

stringfellow commented 7 years ago

@fuhrysteve this is a bit of a bold rework... I've used the 'definitions' scoping to help and this should now work with e.g. https://github.com/jdorn/json-editor/ (see e.g. this fiddle) but unfortunately brutusin/json-forms doesn't support recursive schemas at the moment.

Note this branch includes #33 and #35 and therefore covers both #32 and #34

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling fceea7036f9b3108f12172da902c2a82b98bc5e9 on stringfellow:feature/nested-recursive into 41e50d0bd0a1ede77a5664c7adc73d271079f5d1 on fuhrysteve:master.

stringfellow commented 7 years ago

Hm, think actually this doesn't work with deep nesting. Need to rework to raise all definitions to initial schema dump

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling e184d24abdbb375337c04f46e50b73f77e22ce3f on stringfellow:feature/nested-recursive into 41e50d0bd0a1ede77a5664c7adc73d271079f5d1 on fuhrysteve:master.

fuhrysteve commented 7 years ago

This looks great - the only thing I'm worried about is whether this breaks backwards compatibility with brutusin/json-forms? Obviously it won't support the nesting stuff as you mentioned

stringfellow commented 7 years ago

It won't break backwards compat in that it doesn't remove the ability to render the same schemas as it did - brutusin/json-forms supports $ref, see e.g. https://jsfiddle.net/ho1p0egb/ https://jsfiddle.net/w0gqpvnd/ - though it will still be broken for recursive schemas, as it was already!

stringfellow commented 7 years ago

(also it turns out that other form libs don't handle deep nesting/recursion well.. :( )

fuhrysteve commented 7 years ago

Awesome - that gives me some peace of mind because I have some stuff out there that uses brutisin/json-forms that I'd rather not deal with breaking at the moment

I'll merge whenever you're ready - I'll give it a few hrs at least tho in case you have any last minute changes you want to make

Thanks!!! This is great I couldn't even really find anything to nitpick about haha

stringfellow commented 7 years ago

Haha... amazing, that is the best news I've had in about a month 🙏🏻 If you don't mind having a look over https://github.com/stringfellow/marshmallow-jsonschema/pull/1 as well (which just adds a little tweak to allow some sugar in simple fields) -- I've opened a PR which contains that here: #37