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

Radical idea: Don't do anything about "additionalProperties": false #367

Closed
handrews opened this issue Aug 23, 2017 · 8 comments
Closed

Comments

@handrews
Copy link
Contributor

handrews commented Aug 23, 2017

@jdesrosiers suggested filing an issue for this to include in the vote-a-rama.

One view holds that there is no problem with the existing "additionalProperties" behavior, and the most typical use case that people are trying to solve (catching misspelled / unexpected properties, particularly during development) can be solved outside of the spec.

VOTING FOR THIS MEANS VOTING TO PERMANENTLY CLOSING THE "additionalProperties": false TOPIC

At least to the extent that anything in a draft is permanent. But unless someone comes along with a truly compelling new proposal, requests to revisit it will be denied.

There would be no single solution here, as internal development and testing usage need not be interoperable externally. So an implementation could implement the "strict properties mode" approach (a property must be accounted for somewhere in some subschema) as a runtime option, but it could not be controlled by the schema contents. This is sufficient for pre-deployment testing.

There are other possible outside-of-the-spec runtime approaches, and implementations would be free to implement any, multiple, or none.

@handrews handrews added this to the draft-07 (wright-*-02) milestone Aug 23, 2017
@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.

@epoberezkin
Copy link
Member

epoberezkin commented Aug 23, 2017

I don't think it is radical, really :) It's just maintaining the status quo. For draft-07 it can be the best option, actually.

@handrews
Copy link
Contributor Author

@epoberezkin I was being sarcastic :-P

To be clear, this is NOT about just keeping the status quo for draft-07. This is about closing all of these and declaring it done. One way or another, we are making a decision in this draft.

I will edit the description.

@jdesrosiers
Copy link
Member

@handrews Thanks for adding this. I was going to do it myself, but I'm super busy these days.

One view holds that there is no problem with the existing "additionalProperties" behavior

I wouldn't put it like that. There certainly is a problem with additionalProperties. That's why I don't think we should be tacking on additional functionally to solve the problems that it causes. Instead, we need to get rid of the additionalProperties keyword and find a way to address the need of detecting typos in a way that doesn't violate the core architecture of JSON Schema. Until we have a good solution, I think it's fine to leave it up the individual implementations to solve as they see fit.

@handrews
Copy link
Contributor Author

I wouldn't put it like that.

I confess that was more me talking, I should have been more clear since I flagged you for the concept!

I still consider using "propertyNames" and "enum" together to be the ideal solution (see json-schema-org/json-schema-org.github.io#77 for a detailed example that is far too long to paste here)

I have no intention of removing "additionalProperties" as it implements a critical use case (an object with arbitrary keys whose values must all conform to the same schema). In fact, it is not possible to write the validation meta-schema without it.

And while that use case is simpler than "additionalProperties" in general, I can think of other use cases for it, analogous to Python functions that take some named (or positional) parameters and then gather up the rest in **kwargs.

@json-schema-org json-schema-org deleted a comment from jdesrosiers Aug 27, 2017
@json-schema-org json-schema-org deleted a comment from jdesrosiers Aug 27, 2017
@handrews
Copy link
Contributor Author

For anyone wondering about the deleted comments (assuming the UI keeps noting them), see #373 for a full treatment of the ideas as their own proposal.

@Relequestual
Copy link
Member

I don't really understand why this is an issue. I've commented as such on the opposing issue. Will wait to see if anything explained further.

@handrews
Copy link
Contributor Author

This was actually the most popular option in the vote-a-rama. Others attracted stronger support, but this is the only proposal with zero negative votes. So we are not doing anything in draft-07, but we are focusing on the remaining viable proposals for draft-08.

Since this issue only existed to provide a place for voting, I'm closing it- we do not need an issue to track not doing anything :-)

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