ow.any doesn't support labels about ow

[sindresorhus]

@darkobits My main point is that label() is in the wrong place. If we were to keep it fluent, it should be ow.label('foo')(foo, ow.string); instead, but that doesn't look very good.

[transitive-bullshit]

Yeah, I had overlooked this case; I just wanted to make sure you guys were onboard before submitting a PR. :)

[sindresorhus]

Just an oversight I think.

@transitive-bullshit Thoughts?

[sindresorhus]

Go for it :)

[sindresorhus]

I now come to realize adding .label() to the types were a mistake. The label has nothing to do with the predicate. It should be a top-level thing. Having it on the type also lets the user specify it anywhere in the chain, which results in unreadable code if multiple assertions have the label in different places (I'm sure some users will do that).

It would have made more sense to add it top-level:

ow(foo, 'foo', ow.string);

This would also solve my comment in #92 (comment).

@SamVerschueren @transitive-bullshit Thoughts?

[SamVerschueren]

I agree that after using it for a while, the .label() makes the predicates quite heavy to process mentally. The only thing we should make sure off is that users don't expect ow(foo, 'bar', ow.string) to validate foo.bar. That's what the API of SinonJS does for instance, sinon.spy(foo, 'bar') spies the bar function on the foo object. But I think it makes sense to do it this way.

In the discussion we had about this we had to make a decision because there where many opinions but if we feel like it doesn't really work out, we should refactor it.

[sindresorhus]

The only thing we should make sure off is that users don't expect ow(foo, 'bar', ow.string) to validate foo.bar. That's what the API of SinonJS does for instance, sinon.spy(foo, 'bar') spies the bar function on the foo object.

I don't really see why people would expect that. The person coding it would hopefully read the docs, and the ones reading it would see ow(foo, 'foo'), which doesn't look like property access.

Regardless, with #98, the label would only be needed to be set manually if you want it in production minified in the browser, which I think few would want anyway. So in the end, this doesn't matter much.

[darkobits]

This would definitely be an 🕶addition to this package.

As an aside, I'm not too particular about which route you guys decide to go, though simply adding support for .label() to .any() would be a non-breaking change and, in my humble opinion, be more in line with the current "fluent API" approach that ow uses.

Whether or not you want to keep that design or move to the parameterized approach seems like it may be better suited to a separate discussion.

Either way, keep up the good work! 😸

[alekbarszczewski]

Guys are you planning to do something with this?

[sindresorhus]

PR welcome for my proposal in #89 (comment) :)

[SamVerschueren]

I'm working on this