Skip to content

Passes the "default" function to the schema description#1126

Merged
Marsup merged 2 commits intohapijs:masterfrom
WesTyler:describe_function
Jul 14, 2017
Merged

Passes the "default" function to the schema description#1126
Marsup merged 2 commits intohapijs:masterfrom
WesTyler:describe_function

Conversation

@WesTyler
Copy link
Contributor

@WesTyler WesTyler commented Mar 16, 2017

The .default method already supports providing a function value. This PR expands the information returned by .describe() when the default is a function instead of a string, per #1121 .

Old Behavior

schema.describe();
/*
{ type: 'any', flags: { default: 'Generate random number' } }
*/

New Behavior

schema.describe();
/*
{ type: 'any', flags: { default: {
    description: 'Generate random number',
    function: randomNumber
} } }
*/

@Marsup
Copy link
Collaborator

Marsup commented Mar 22, 2017

This feels like a breaking change. I'll keep this one for next.

@Marsup Marsup self-assigned this Mar 22, 2017
@Marsup Marsup added breaking changes Change that can breaking existing code feature New functionality or improvement labels Mar 22, 2017
@Marsup Marsup self-requested a review March 22, 2017 22:05
@Marsup Marsup added this to the 11.0.0 milestone Mar 22, 2017
@WesTyler
Copy link
Contributor Author

WesTyler commented Apr 3, 2017

Oh hey @Marsup , if this is the only breaking change slated for a v11 update I'm all for reworking this to not be breaking. Maybe something like this behavior instead?

schema.describe();
/*
{ type: 'any', flags: { 
    default: 'Generate random number', // no change vs current behavior
    defaultFunction: randomNumber // new property, shouldn't be breaking
} }
*/

@WesTyler WesTyler mentioned this pull request Apr 25, 2017
Merged
6 tasks
@Marsup
Copy link
Collaborator

Marsup commented Jun 30, 2017

@WesTyler mind rebasing this one ?

@WesTyler
Copy link
Contributor Author

WesTyler commented Jul 6, 2017

Sure thing. It's on my to-do list for today :)

@Marsup Marsup merged commit ad0160b into hapijs:master Jul 14, 2017
@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.

Reviewers

@Marsup Marsup Awaiting requested review from Marsup

Assignees

@Marsup Marsup

Labels

breaking changes Change that can breaking existing code feature New functionality or improvement

Projects

None yet

Milestone

11.0.0

Development

Successfully merging this pull request may close these issues.

2 participants

@WesTyler @Marsup