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

Opt-in to allowing unknown keys in a shape() #116

Open
job13er opened this issue Mar 13, 2017 · 9 comments
Open

Opt-in to allowing unknown keys in a shape() #116

job13er opened this issue Mar 13, 2017 · 9 comments

Comments

@job13er
Copy link
Contributor

job13er commented Mar 13, 2017

Currently, if a property defined as a shape() is given an object that meets that shape, but has additional properties, it will error because of the existence of unknown keys. Sometimes, consumers may want to specify what they expect, but not be upset if the user provides additional properties that they will simply ignore. It would be nice if we could support this by maybe accepting a second options block in the shape() method, or a single, boolean second argument for the purpose of enabling additional properties to be accepted:

stuff: PropTypes.shape({
  foo: PropTypes.string,
  bar: PropTypes.string
},
{
  additionalProperties: true
})

or

stuff: PropTypes.shape({
  foo: PropTypes.string,
  bar: PropTypes.string
}, true)
@sglanzer-deprecated
Copy link
Contributor

Is there a use case for the current behavior (error on unknown keys)? I'd be willing to advocate for additional properties to be allowed as the default

@job13er
Copy link
Contributor Author

job13er commented Mar 14, 2017

I wouldn't mind allowing additional properties by default, but that's a breaking change. I'd vote we first add the option to allow people to opt-in, then decide when we make our next breaking change if we want to change the default.

@sglanzer-deprecated
Copy link
Contributor

Totally on board with that, just wanted to see if there was a reason explicitly for the previous behaviour

@job13er
Copy link
Contributor Author

job13er commented Mar 14, 2017

Not that I know of.

@gustaff-weldon
Copy link

gustaff-weldon commented Apr 10, 2017

I had the same problem, I wondered about making shape configurable, but given my 0-knowledge of codebase (and shortage of time), I went ahead and copied shape to iface prop type and removed unnecessary checks for additional properties.

gustaff-weldon@78fc766

Let me know if you'd like a PR with that (I know it's a duplication, but it does the job for me)

@gustaff-weldon
Copy link

Any progress on that? Have you guys decided if that will be a breaking or opt-in change?

@job13er
Copy link
Contributor Author

job13er commented Jul 18, 2017

@gustaff-weldon Sorry, no progress that I'm aware of. I believe most people who had an interest in this issue have moved on to different companies. I'm not sure who will be maintaining this going forward. @cstolli may have some insight...

@sunglam
Copy link

sunglam commented May 20, 2018

@sglanzer I think it must be there. There are a lot of use cases to do so.

@sglanzer
Copy link

@sunglam I'd agree that there are good use cases for it; I'm actually at a new company though and while we use prop-types this currently isn't a priority for me, nor do I direct the open source effort for Ciena ;)

The best I can do at this moment is agree with you :)

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

5 participants