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

"Describe" behavior changes for "implicit" Any with extended Joi #1067

Closed
WesTyler opened this issue Dec 15, 2016 · 8 comments
Closed

"Describe" behavior changes for "implicit" Any with extended Joi #1067

WesTyler opened this issue Dec 15, 2016 · 8 comments
Assignees
Labels
bug Bug or defect
Milestone

Comments

@WesTyler
Copy link
Contributor

Context

  • node version: 4.5.0
  • joi version: 10.0.6
  • environment (node, browser): node
  • used with (hapi, standalone, ...): joi-date-extensions

What are you trying to achieve or the steps to reproduce ?

I know that using Joi.any().valid('value') is preferable over using Joi.valid('value'), but the behavior of .describe() is changing between the two cases when Joi is extended.

This change in describe is less than ideal for Felicity since we built off of describe() and hope to support extensions soon.

I'm using joi-date-extensions here, but it appears to be related to .extend rather than the extension itself.

const BaseJoi = require('joi');
const JoiDate = require('joi-date-extensions');
const ExtendedJoi = BaseJoi.extend(JoiDate);

// What I wish users would do, but cannot guarantee:
ExtendedJoi.any().valid('type').default('type').describe();
/*
{ type: 'any',
  flags: { allowOnly: true, default: 'type' },
  valids: [ 'type' ] }
*/

// How I expect the extended Joi to continue to behave:
BaseJoi.valid('type').default('type').describe();
/*
{ type: 'any',
  flags: { allowOnly: true, default: 'type' },
  valids: [ 'type' ] }
*/

// Actual extended Joi behavior. Flags and valids are dropped
ExtendedJoi.valid('type').default('type').describe();
/*
{ type: 'any' }
*/
@WesTyler
Copy link
Contributor Author

I'm currently looking into this, but wanted to submit the issue in the meantime in case anyone has input on the issue :)

@WesTyler
Copy link
Contributor Author

It looks like ExtendedJoi.valid('type').describe() invokes the root.describe here, and the ternary is defaulting to the false case, which is a new Any():
const schema = arguments.length ? root.compile(arguments[0]) : any;

@WesTyler
Copy link
Contributor Author

Ok, it looks like it may just be too bad.

When BaseJoi.valid('type') is called, BaseJoi is returning an instance of (new Any()).clone() (here), which is why the "implicit" any behavior works.

But! When ExtendedJoi.valid('type') is called, the root class is returned (here and here). That's why when .describe() is called, it behaves as BaseJoi.describe() instead of any().describe().

I'm going to leave this open for visibility and just in case I misunderstood something, but I think it is a dead end without re-implementing extend.

@WesTyler
Copy link
Contributor Author

Closing this out now. Not sure why I felt the need to leave it open for so long.

@Marsup
Copy link
Collaborator

Marsup commented Mar 17, 2017

I'm not sure what to do with this, this is definitely a bug, but I don't yet see a way out of it.

@Marsup
Copy link
Collaborator

Marsup commented Mar 17, 2017

OK, found a way, never talk too soon :)
I'll get on it after diner, maybe expect a new release in the week-end if I'm motivated.

@Marsup Marsup reopened this Mar 17, 2017
@Marsup Marsup self-assigned this Mar 17, 2017
@Marsup Marsup added the bug Bug or defect label Mar 17, 2017
@WesTyler
Copy link
Contributor Author

WesTyler commented Mar 17, 2017

Oh, nice! :D

I couldn't find a way out of it either. My only "solution" was essentially a breaking change rewrite of how extend works... So I'm glad you found a way!

@Marsup
Copy link
Collaborator

Marsup commented Mar 17, 2017

It doesn't seem to break anything but who knows :D

@Marsup Marsup added this to the 10.3.0 milestone Mar 18, 2017
@Marsup Marsup closed this as completed in 3b34640 Mar 18, 2017
@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
bug Bug or defect
Projects
None yet
Development

No branches or pull requests

2 participants