Skip to content

array().single() #498

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

Closed
hueniverse opened this issue Nov 26, 2014 · 25 comments
Closed

array().single() #498

hueniverse opened this issue Nov 26, 2014 · 25 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@hueniverse
Copy link
Contributor

A different take on #474

Allows an array value to pass if the value would be a valid array element. It conversion is enabled, turns the item into an array. Joi.array().includes(Joi.number()).single() will match [1] and 1 and will result in [1] in both cases.

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

Still not convinced about the usefulness of such construct, it seems like a poorly designed frontend, am I missing something here ? If you intend to pass an array just do it, and the accepted pattern for it is to put a[]=1 even for a single value.

@hueniverse
Copy link
Contributor Author

Forget about front end. I have these rules in the hapi schema for every field that accepts a value or array of values. I really don't care about the query string use case, but this does address it.

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

Fair point.

@Marsup Marsup self-assigned this Nov 26, 2014
@Marsup Marsup added this to the 5.0.0 milestone Nov 26, 2014
Marsup added a commit that referenced this issue Nov 26, 2014
@Marsup Marsup closed this as completed Nov 26, 2014
@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

Renamed it to allowSingle() because single() looked like a constraint.

@hueniverse
Copy link
Contributor Author

It's both, like uppsercase() so single() would be more consistent.

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

uppercase() is both a check and a conversion depending on convert, I only see the conversion part on single().

@hueniverse
Copy link
Contributor Author

How is that different? single() should only verify if convert is false.

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

There's no possible failure to expect from single(), there's no assertion ever, so I don't see it the same way.

@hueniverse
Copy link
Contributor Author

There could: the single item provided does not match the allowed array items.

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

This is then referring to the includes() or excludes() tests, not really the single.

@hueniverse
Copy link
Contributor Author

I think the error would be more useful if it referred to the single item state. Does the current solution address an array of arrays?

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

The current solution wraps into an array if it's not, and there's probably no way to know about an array of arrays in one pass.

@hueniverse
Copy link
Contributor Author

I think that's part of the problem in this debate.

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

This kind of concern was not part of the original issue.

@hueniverse
Copy link
Contributor Author

Might be worth to reconsider fixing or removing for now. The way it should work is by checking if the value matches the plain rule (the array rules) and if not, check if it would pass as a single array item, and if yes, then convert to an array if convert is on. I have not looked into how to implement it.

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

There are only so many days to hapi 8, that'll probably be the last one to land.

@hueniverse
Copy link
Contributor Author

Your call.

@Marsup
Copy link
Collaborator

Marsup commented Nov 27, 2014

The "array of arrays" is dealt with, not so sure about the error message though, it's a lot of added complexity just to change from "position 0 does not match..." to "your single item doesn not match...". The language doesn't seem to support composability of errors yet so...

@hueniverse
Copy link
Contributor Author

Don't know. When we didn't do this correctly in the past (many times) it always came back to bite us in the ass.

@Marsup
Copy link
Collaborator

Marsup commented Nov 28, 2014

I'm wondering about the message to show in the case of array of arrays, should it be something like "the single item doesn't match..." or "value at position 0 doesn't match...", in essence, should it be the 1st pass error or the 2nd one ?

@hueniverse
Copy link
Contributor Author

Ideally the first pass, but not worth complexity over.

@Marsup
Copy link
Collaborator

Marsup commented Nov 29, 2014

Should be good to go now.

@hueniverse
Copy link
Contributor Author

Was the name changed back to single()?

@Marsup
Copy link
Collaborator

Marsup commented Nov 29, 2014

Yes.

@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

2 participants