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

array.unique doesn't validate duplicate dates #497

Closed
gaastonsr opened this issue Nov 26, 2014 · 7 comments
Closed

array.unique doesn't validate duplicate dates #497

gaastonsr opened this issue Nov 26, 2014 · 7 comments
Assignees
Labels
breaking changes Change that can breaking existing code bug Bug or defect
Milestone

Comments

@gaastonsr
Copy link
Contributor

Should not array.includes(Joi.date()).unique() validate duplicate dates?

@Marsup Marsup added the bug Bug or defect label Nov 26, 2014
@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

Like said in the documentation, only literals work currently, but looks like a bug to me.

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

@hueniverse mind if I change that behavior ? Tests seem to explicitly deny that, was it for a reason ?

@hueniverse
Copy link
Contributor

The test confirms the docs. It was set this way to avoid expensive deep comparisons of objects. Whatever the change, it needs to perform well and to be well defined. Does unique means identical references or deep equality?

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

I was going for deep equality. This needs to be made explicit that there will be a performance penalty on objects indeed.

@hueniverse
Copy link
Contributor

Hoek has utils for deep comparisons now.

@Marsup
Copy link
Collaborator

Marsup commented Nov 26, 2014

Yes, already did the code with that, just needs tests now.

@Marsup Marsup added the breaking changes Change that can breaking existing code label Nov 26, 2014
@Marsup Marsup self-assigned this Nov 26, 2014
@Marsup Marsup added this to the 5.0.0 milestone Nov 26, 2014
@Marsup Marsup closed this as completed in 0dff718 Nov 26, 2014
@gaastonsr
Copy link
Contributor Author

Cool. Thanks.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking changes Change that can breaking existing code bug Bug or defect
Projects
None yet
Development

No branches or pull requests

3 participants