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

$spread for including more properties #374

Closed
handrews opened this issue Aug 27, 2017 · 10 comments
Closed

$spread for including more properties #374

handrews opened this issue Aug 27, 2017 · 10 comments

Comments

@handrews
Copy link
Contributor

handrews commented Aug 27, 2017

This is a proposal from @ruifortes. All words after this intro paragraph are his, copied from json-schema-org/json-schema-org.github.io#77 (comment) Note that this is sort-of a simplified version of #321.


Why not just use something like spread in js?

Instead of creating complicated semantics for the json-schema itself it could be assumed that some basic operations would be possible in simple json like "$ref" is.
"#ref" has nothing to do with the schema itself, just json.
So, why not use a similar approach using "$spread" like the js obj2 = {...obj1, otherProp, yetAnotherOne} but declarative.

If you have to basic shemas foo and bar

{
    description: 'foo schema',
    type: 'object',
    properties: {
        foo1: {type: 'number'},
        foo2: {type: 'boolean'}
    }
}
{
    description: 'bar shema' ,
    type: 'object'
    properties: {
        bar1: {type: 'string'},
        bar2: {type: 'boolean'}
    }
}

you could extend foo like:

{
    description: 'foo and more shema' ,
    type: 'object'
    properties: {
        more1: {type: 'string'},
        more2: {type: 'boolean'},
        $spread: {$ref: 'pathto/foo#properties'}
    }
}

eventually $spread position in would be relevant for overriding props.

To create a foobar schema that extends both you could:

{
    description: 'foobar and more shema' ,
    type: 'object'
    properties: {
        foobarSpecific: {type: 'string'},
        $spread: [
            {$ref: 'pathto/foo#properties'},
            {$ref: 'pathto/baar#properties'}
        ]
    }
}

The idea would be that json-schema would be a simple schema definition without fancy inheritance semantics and there would be some json (object) declarative manipulations mechanism like $ref and something like $spread could be another one

@handrews
Copy link
Contributor Author

VOTE-A-RAMA!!!

It's time to gauge community support for all re-use/extension/additionalProperties proposals and actually make some decisions for the next draft.

Please use emoji reactions ON THIS COMMENT to indicate your support.

  • You do not need to vote on every proposal
  • If you have no opinion, don't vote- that is also useful data
  • If you've already commented on this issue, please still vote so we know your current thoughts
  • Not all proposals solve exactly the same problem, so we may end up implementing more than one

This is not a binding majority-rule vote, but it will be a very significant input into discussions.

Here are the meanings for the emojis:

  • Celebration / Hooray / Tada!: I support this so strongly that I want to be the primary advocate for it
  • Heart: I think this is an ideal solution
  • Smiley face: I'd be happy with this solution
  • Thumbs up: I'd tolerate this solution
  • Thumbs down: I'd rather we not do this, but it wouldn't be the end of the world
  • Frowny face: I'd be actively unhappy, and may even consider other technologies instead

If you want to explain in more detail, feel free to add another comment, but please also vote on this comment.

The vote should stay open for several weeks- I'll update this comment with specifics once we see how much feedback we are getting and whether any really obvious patterns emerge.

@Relequestual
Copy link
Member

I guess this is sort of how I thought $merge would work before reading. This could be a simpliar syntax for $merge. Thoughts @handrews ?

@erosb
Copy link

erosb commented Aug 31, 2017

At the first glance my problem with this is that this keyword gives confusing semantics for "required". Eg. if "Foo schema" declares "required": ["foo1"] then "foo1" would be required in "Foo schema" but not in "FooBar schema". This would be very confusing for schema authors.

So my concern is the unclear way how keywords that affect object properties but they are not inside "properties" work.

Thumbs down for this. I will support it in my implementation if this proposal makes into draft-7, but I'm not very happy about it.

@handrews
Copy link
Contributor Author

@erosb I consider it a critical feature of any re-use system that re-using a set of properties allows but does not force applying the same requirements. It is very common for me to need an object that is writeable in one context but read-only in others.

@handrews
Copy link
Contributor Author

@Relequestual I find this to basically be a simpler alternative to "$merge" that narrowly targets the objections that people have with "additionalProperties" and that draws from JavaScript idioms for syntax. All of those things are good in my book.

At this point, I think my primary concern is with being able to turn off whatever feature people insist on adding for this.

@erosb
Copy link

erosb commented Aug 31, 2017

Well, I still think that being able to create instances which are valid by the child schema, while invalid by the parent schema will cause confusions (and confusion isn't good, I'm quite sure about that if I implement the spec this way then a number of bugreports will be submitted for me, telling me that the implementation has a bug).

The reason why I'm not strongly against this proposal is that is that
a) it works around the problem of "additionalProperties"
b) JSON Schema was never similar to object-oriented type systems , it is more like a type system built from the foundations of first-order logic (I mean the anyOf / allOf / not / etc .. constructs), so even if $spread violates LSP, it doesn't make the big picture any worse.

@handrews
Copy link
Contributor Author

Well, I still think that being able to create instances which are valid by the child schema, while invalid by the parent schema will cause confusions

I'm 100% with you on this, hence wanting to be able to turn it off / write re-usable schemas that do not allow splicing into other schemas, by whatever mechanism is chosen.

So yes, in terms of its interaction with "additionalProperties", "$spread" is monkey-patching rather than inheritance. And if you're not dealing with "additionalProperties' (with any non-true schema, not just false), there's no need for "$spread" at all. "allOf" is sufficient and fits in the current theoretical model.

So... yeah, monkey-patching, not inheritance. And I'm not convinced JSON Schema needs either (for code generation concerns, we have a separate code generation vocabulary under development, so I don't regard that as a use case for validation).

@handrews
Copy link
Contributor Author

handrews commented Sep 2, 2017

@erosb (and anyone else) see #388 for a possible approach to providing one of these solutions as an experimental opt-in extension

At the moment it's my best idea for moving forward while still preserving a theoretically consistent option for those of us who find that important.

@handrews handrews added this to the draft-08 milestone Sep 11, 2017
@epoberezkin
Copy link
Member

How would property named "$spread" be possible? Maybe it (or some other keyword) should be one level higher?...

@handrews
Copy link
Contributor Author

At the end of the vote-a-rama, I said that I would consolidate these issues to focus the discussion for draft-08. I've filed #515 which outlines the two possible competing approaches. It's pretty abstract, but once we choose an approach, deciding which exact keyword and behavior we want should be less controversial. Therefore I'm closing this in favor of #515.

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

No branches or pull requests

4 participants