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

support factory-function in .default() ? #363

Closed
Bartvds opened this issue Jun 13, 2014 · 25 comments
Closed

support factory-function in .default() ? #363

Bartvds opened this issue Jun 13, 2014 · 25 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@Bartvds
Copy link
Contributor

Bartvds commented Jun 13, 2014

As the readme notes using an reference-type as the value for default makes the same instance shared by all invocations, which is not that useful.

Would it be an idea to allow a function as value, to be called when a new default value is needed so it can produces a new instance?

@hueniverse
Copy link
Contributor

Nah. If you need unique instances of a default object you should move that logic to code after validation.

@hueniverse hueniverse self-assigned this Jun 13, 2014
@Starefossen
Copy link
Contributor

This would be useful for things like:

var schema = Joi.object().keys({
  created: Joi.date().default(Date.now)
});

@hueniverse hueniverse reopened this Jul 15, 2014
@jedireza
Copy link

👍 I had the same use case @Starefossen shared.

@pward123
Copy link

pward123 commented Sep 5, 2014

+1 same use case as @Starefossen

@silas
Copy link

silas commented Nov 18, 2014

+1

@hueniverse hueniverse removed their assignment Nov 18, 2014
@satazor
Copy link

satazor commented Jan 1, 2015

+1

@Marsup
Copy link
Collaborator

Marsup commented Jan 1, 2015

Is the only use case to accept 'now' as is already the case in min and max ?

@silas
Copy link

silas commented Jan 1, 2015

No, I certainly have other use cases, such as special cases of Date.now (such as 00:00 of today), uuids, expire times (30 minutes from now), etc..

Obviously it's easy to do this in code, but it would be nice to be able to just pass default a function.

@satazor
Copy link

satazor commented Jan 2, 2015

@Marsup I got an use case for now but I've a more complex case where I need to infer a default value based on the contents of another property:

{
   artists: [{ name: 'Madonna', role: 'main' }]
   display_artist: 'Madonna'
}

The display_artist is a "required" field that defaults to a string built manually from the artists array. If I could pass a function in default() it would look like this:

{
   artists: Joi.array()....,
   display_artist: Joi.string().trim().default(function (obj) {
     return obj.artists.reduce(function (previousVal, currentVal) {
         return previousVal += currentVal.name + ' ';
     }, '').trim();
   })
}

At the moment I'm doing the complex "default" stuff like this after Joi validation.

@bleupen
Copy link

bleupen commented Jan 14, 2015

+1

@briansorahan
Copy link
Contributor

+1 I could benefit from two of the use cases mentioned already: Date.now and uuid.

@rtyer
Copy link

rtyer commented Feb 4, 2015

+1 Same here on date and uuid generation.

@pekeler
Copy link

pekeler commented Feb 9, 2015

+1

1 similar comment
@wspringer
Copy link

+1

@Marsup Marsup self-assigned this Feb 15, 2015
@Marsup Marsup added this to the 6.0 milestone Feb 15, 2015
@nlf nlf closed this as completed in ba40cd5 Feb 18, 2015
Marsup added a commit that referenced this issue Feb 18, 2015
allow passing a method to .default(), for #363
@pekeler
Copy link

pekeler commented Feb 18, 2015

Nice :)

@bleupen
Copy link

bleupen commented Feb 18, 2015

so is this a breaking change for existing joi.func().default(fn) schema expressions?

@Marsup
Copy link
Collaborator

Marsup commented Feb 18, 2015

Thanks @bleupen, this one slipped through the reviews, we'll fix that before release.

@Marsup
Copy link
Collaborator

Marsup commented Feb 18, 2015

Fixed by #555.

Marsup added a commit that referenced this issue Feb 18, 2015
add some additional guards and tests for #363
@bleupen
Copy link

bleupen commented Feb 18, 2015

I'm very hapi you guys are working on this one, although something feels wrong about the modal nature of default() based on the presence of a description attribute. It violates the principal of least surprise imo. Have you considered adding a factory() instead and preserve the existing behavior of default()?

Sent from my iPhone

On Feb 18, 2015, at 3:31 PM, Nicolas Morel notifications@github.com wrote:

Fixed by #555.


Reply to this email directly or view it on GitHub.

@Marsup
Copy link
Collaborator

Marsup commented Feb 18, 2015

I don't understand, this is not a breaking change, default will continue to work as it is today, it's just an improvement.

@bleupen
Copy link

bleupen commented Feb 18, 2015

It’s really only an issue when the item type is joi.func() (i use this feature a lot to sanitize drivers). In this case, if I understand correctly, adding a second description parameter to default() is what throws joi into "higher order function mode”. While non-breaking, it feels surprising to me. Perhaps I’m misreading the code.

On Feb 18, 2015, at 3:31 PM, Nicolas Morel notifications@github.com wrote:

Fixed by #555 #555.


Reply to this email directly or view it on GitHub #363 (comment).

@Marsup
Copy link
Collaborator

Marsup commented Feb 18, 2015

That or having a description property on your function. Why is that surprising ?

@bleupen
Copy link

bleupen commented Feb 18, 2015

Because whether a function is a factory or not is orthogonal to its description. Not a huge deal, I can definitely work with whats here.

On Feb 18, 2015, at 4:37 PM, Nicolas Morel notifications@github.com wrote:

That or having a description property on your function. Why is that surprising ?


Reply to this email directly or view it on GitHub #363 (comment).

@Marsup
Copy link
Collaborator

Marsup commented Feb 18, 2015

I think it's clearer this way, I don't want a debate about whether default or factory wins if both exists :)

Joke aside, the description is more of a side-effect that's needed if we want to keep the ability to describe validations. I see it the other way around, the description is needed because you provided a function, it doesn't make the function special, except in the Joi.func() case you highlighted.

@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