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

Correct way to write an "identity" extension? #1000

Closed
LudwikJaniuk opened this issue Oct 6, 2016 · 14 comments
Closed

Correct way to write an "identity" extension? #1000

LudwikJaniuk opened this issue Oct 6, 2016 · 14 comments
Assignees
Labels
bug Bug or defect
Milestone

Comments

@LudwikJaniuk
Copy link
Contributor

(by "identity" I mean "one that changes nothing", similar to the identity function)

Context

  • node version:6
  • joi version:9
  • environment (node, browser):node
  • used with (hapi, standalone, ...):standalone
  • any other relevant information:I'm extending joi

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

I'm trying to do a minimal "identity" extension to joi that would replace an existing data type, but not change it in any way, so I'm using it as the base.

var joi = require("joi");
joi = joi.extend({
    name: "object",
    base: joi.object(),
});

Which result you had ?

joi.object() seems to be behaving slightly differently, it has something to do with .options() and .keys() and allowUnknown changing their behavior. I will construct a minimal example demonstrating this if it's not obvious what I'm doing wrong.

What did you expect ?

I expected the new joi.object() to work exactly as before, and change nothing in how the new joi object behaves. Was that assumption correct? Is this how you would write an "identity" extension?

@Marsup
Copy link
Collaborator

Marsup commented Oct 6, 2016

Not sure I understand, identity is joi.any() for me.

@LudwikJaniuk
Copy link
Contributor Author

No, what I mean is that I want to write an extension that would be a "copy"
of joi.object, that would replace it, and work exactly like it. And I think
I'm doing something wrong.

On Thu, Oct 6, 2016, 12:33 Nicolas Morel notifications@github.com wrote:

Not sure I understand, identity is joi.any() for me.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1000 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGAbGe3wZSsbwoGhPARcl6QEP9eV54ctks5qxM6JgaJpZM4KPx6L
.

@Marsup
Copy link
Collaborator

Marsup commented Oct 7, 2016

I'll wait for your example to see if there's anything wrong with type overriding, I think it's tested.

@LudwikJaniuk
Copy link
Contributor Author

Ok, will post it soon.

On Fri, Oct 7, 2016, 02:23 Nicolas Morel notifications@github.com wrote:

I'll wait for your example to see if there's anything wrong with type
overriding, I think it's tested.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1000 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AGAbGSNojpaUkMO6vUJ4Ls8I16d92QOJks5qxZEEgaJpZM4KPx6L
.

@LudwikJaniuk
Copy link
Contributor Author

Here is a minimal example. This validation fails with "a" is not allowed:

var joi= require("joi");
joi = joi.extend({ name: "object", base: joi.object() });

var schema_a = joi.object({ a: joi.number(), }).options({allowUnknown: false});
var schema_b = schema_a.keys({ b: joi.any(), });

var subject = {
  a: 3,
  b: 4,
};

joi.assert(subject, schema_b);

But this one succeeds (line 2 commented out):

var joi= require("joi");
//joi = joi.extend({ name: "object", base: joi.object() });

var schema_a = joi.object({ a: joi.number(), }).options({allowUnknown: false});
var schema_b = schema_a.keys({ b: joi.any(), });

var subject = {
  a: 3,
  b: 4,
};

joi.assert(subject, schema_b);

I assumed the new joi.object would be just like the old one, and that's why I'm surprised by this behaviour and it's breaking my tests

@LudwikJaniuk
Copy link
Contributor Author

@Marsup But maybe I'm just doing something wrong, and this is not the correct way to write a "copy" of joi.object()?

@Marsup
Copy link
Collaborator

Marsup commented Oct 12, 2016

You're using an api that's a shorthand to joi.object().keys() (https://github.com/hapijs/joi/blob/master/lib/index.js#L74) that's not really part of the object type, when extending you're not bringing this with you. Extending the constructor is not yet possible, I might consider this in the future if there's enough demand for it.

@Marsup Marsup added the request label Oct 12, 2016
@LudwikJaniuk
Copy link
Contributor Author

Oh, so when specifying base, the methods don't follow along into the new type? That's a shame, but now I have a better understanding of why this was failing. Any suggestions on how to work around this? Could I create a new type that would have new rules + the functions of an existing type? If not, I guess the plan B if there is no better option, would be to create a new type with the extension rules, and use joi.and to test agains joi.object and this one.

@Marsup
Copy link
Collaborator

Marsup commented Oct 13, 2016

Just use Joi.object().keys() instead of the shorter version in the meantime.

@LudwikJaniuk
Copy link
Contributor Author

Oh, so it's only special features of the constructor that don't get carried over. Now I understand, and I can easily work around this. Thanks!

I'm satisfied and could close this issue as for me, but you said you'd maybe implement this in the future, so perhaps you want to keep it open?

@Marsup
Copy link
Collaborator

Marsup commented Oct 13, 2016

Indeed this could be an interesting feature so I'll keep this open. Even mark it as open for contributors.

@talyssonoc
Copy link

Do you intent to support it? If so, I can try to send a PR to add the option to pass arguments to custom extensions.

@Marsup
Copy link
Collaborator

Marsup commented Apr 12, 2019

Arguments are in my plans but it needs careful design because of inheritance, I didn't have time to think about it so far.

@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
bug Bug or defect
Projects
None yet
Development

No branches or pull requests

3 participants