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

Object shape validator #92

Closed
sindresorhus opened this issue Jun 7, 2018 · 20 comments
Closed

Object shape validator #92

sindresorhus opened this issue Jun 7, 2018 · 20 comments
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Milestone

Comments

@sindresorhus
Copy link
Owner

sindresorhus commented Jun 7, 2018

Issuehunt badges

Would be useful to be able to validate the shape of objects, like PropTypes.shape

I imagine it could be like:

ow(input, ow.object.shape({
	color: ow.string,
	height: ow.number.greatherThan(10)
}))

@SamVerschueren @transitive-bullshit Thoughts?

samverschueren earned $60.00 by resolving this issue!

@transitive-bullshit
Copy link
Contributor

transitive-bullshit commented Jun 7, 2018

I agree that this would be useful, though I'm not sure I'm in favor of adding it to the core API since unlike its usage in PropTypes, you can accomplish the same thing with multiple ow statements.

ow(input, ow.object)
ow(input.color, ow.string)
ow(input.height, ow.number.greatherThan(10))

This isn't possible with PropTypes which is one reason they added PropTypes.shape.

My main argument against it would be to keep the API surface as small as possible, but I do find myself using this pattern quite often especially for validating configuration parameters opts.

Take this concrete example where I'm using ow for this purpose. It'd be really awkward if I tried to use a combination of ow.object.shape and the upcoming optional predicate to achieve what is more clearly defined by multiple ow statements.

I could go either way, though, since I'm sure there are use cases where it'll still be useful to peeps. Just my 2 cents 👍

@sindresorhus
Copy link
Owner Author

I think it should be in core. I very often have APIs with an options-object and I want to quickly validate the options. Having to do separate ow calls for each property would be verbose and annoying.


I think this:

ow(target, ow.object.label('target').shape({
	width: ow.number.positive.integer,
	height: ow.number.positive.integer,
	data: ow.any(ow.uint8Array, ow.uint8ClampedArray)
}))

is better than this:

ow(target, ow.object.label('target'))
ow(target.width, ow.number.positive.integer.label('target.width'))
ow(target.height, ow.number.positive.integer.label('target.height'))
ow(target.data, ow.any(ow.uint8Array, ow.uint8ClampedArray))

We also wouldn't need label() for the object properties as they can be inferred.

@transitive-bullshit
Copy link
Contributor

transitive-bullshit commented Jun 7, 2018

We also wouldn't need label() for the object properties as they can be inferred.

💯

Sounds good to me :)

@SamVerschueren
Copy link
Collaborator

We discussed this somewhere else, can't find it though. I believe our initial suggestion was just

ow(target, {
	width: ow.number.positive.integer,
	height: ow.number.positive.integer,
	data: ow.any(ow.uint8Array, ow.uint8ClampedArray)
})

So just passing an object as second parameter would do the trick. I see positive sides to both ideas.

@sindresorhus
Copy link
Owner Author

sindresorhus commented Jun 11, 2018

@SamVerschueren I like that shorthand, but there's no way to use a label or to make it optional (when that is added) with that...

@sindresorhus
Copy link
Owner Author

PropTypes.shape() doesn't throw on additional unspecified keys. They have PropTypes.exact() for that. We need to handle both cases.

@transitive-bullshit
Copy link
Contributor

Just a note; the shape vs exact semantics seem very related to the optional operator and may have some overlap in implementation.

@sindresorhus
Copy link
Owner Author

@transitive-bullshit The optional operator would only be useful for known optional properties. What about unknown properties?

@IssueHuntBot
Copy link

@issuehuntfest has funded $60.00 to this issue. See it on IssueHunt

@sindresorhus sindresorhus added this to the 1.0.0 milestone Jan 10, 2019
@mhaagens
Copy link

Using this for simple input validation, unfortunately only throws on the first error, but that's all I need to make sure there are no erroneous inputs being passed.

const matchShape = (source, shape) =>
  new Promise((resolve, reject) => {
    for (let key of Object.keys(source)) {
      try {
        ow(source[key], shape[key]);
      } catch (e) {
        return reject(e);
      }
    }
    resolve();
});

Usage:

const obj = {
  id: "Gozxoqqy79"
}

await matchShape(obj, {
  id: ow.string
}).catch(e => {
  console.log(e);
});

@transitive-bullshit
Copy link
Contributor

This is a nice wrapper @mhaagens 😄 Any reason it has to be asynchronous?

@mhaagens
Copy link

Thanks! :) Doesn't have to be at all. Just like to be explicit about returning a promise when using async/await and I like the syntax better than callbacks :).

@transitive-bullshit
Copy link
Contributor

@mhaagens I meant that there's no need to use async / Promise or callbacks in this case since everything's synchronous.

const matchShape = (source, shape) => {
  for (const key of Object.keys(source)) {
    ow(source[key], shape[key]);
  }
}

matchShape(obj, { id: ow.string })

@mhaagens
Copy link

@transitive-bullshit Ah, yeah you're right!

@SamVerschueren
Copy link
Collaborator

SamVerschueren commented Jan 25, 2019

I want to start working on this. But before I do, what API are we going to use now?

// Allows extra properties
ow(target, ow.object.shape({
	width: ow.number.positive.integer,
	height: ow.number.positive.integer,
	data: ow.any(ow.uint8Array, ow.uint8ClampedArray)
}));

And

// Doesn't allow extra properties
ow(target, ow.object.exact({
	width: ow.number.positive.integer,
	height: ow.number.positive.integer,
	data: ow.any(ow.uint8Array, ow.uint8ClampedArray)
}));

Or exactShape?

@sindresorhus
Copy link
Owner Author

Or exactShape?

👍


When you only see .shape() in some code, is it clear enough that it allows additional properties? For me, it's not. How about partialShape() or looseShape()? I prefer the former. Open to other naming suggestions.

@SamVerschueren
Copy link
Collaborator

The partialShape is already done 👌. When I started on exactShape, I wasn't sure how it should handle certain use cases.

Does exactShape mean it should be exact in both directions?

ow({unicorn: '🦄'}, ow.object.exactShape({
	unicorn: ow.string,
	rainbow: ow.string
}));
ow({unicorn: '🦄', rainbow: '🌈'}, ow.object.exactShape({
	unicorn: ow.string
}));

Should both of these checks fail, or only one of them? It has quite some impact on the implementation.

@sindresorhus
Copy link
Owner Author

Exact should mean "exact" in that the shape should have all the same properties as the input object.

@sindresorhus
Copy link
Owner Author

Should both of these checks fail

Yes

@IssueHuntBot
Copy link

@sindresorhus has rewarded $54.00 to @SamVerschueren. See it on IssueHunt

  • 💰 Total deposit: $60.00
  • 🎉 Repository reward(0%): $0.00
  • 🔧 Service fee(10%): $6.00

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🎁 Rewarded on Issuehunt This issue has been rewarded on Issuehunt help wanted
Projects
None yet
Development

No branches or pull requests

5 participants