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

Error when .rename() from undefined #614

Closed
JbIPS opened this issue Mar 25, 2015 · 20 comments
Closed

Error when .rename() from undefined #614

JbIPS opened this issue Mar 25, 2015 · 20 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@JbIPS
Copy link

JbIPS commented Mar 25, 2015

Hi,
here's what I'm trying to do :
I've got this Joi schema

var validateQuery = core.Joi.object().keys({
  clientId: core.Joi.string().required(),
  redirectUri: core.Joi.string().required().regex(/^[a-z0-9]*:\/\/[^*]*/i),
  responseType: core.Joi.string().required().valid(['code', 'token']),
}).rename('client_id', 'clientId', {override: true})
  .rename('redirect_uri', 'redirectUri', {override: true})
  .rename('response_type', 'responseType', {override: true});

and I'm validating this object

  redirectUri: 'http://whatever.com',
  responseType: 'token',
  clientId: '240f17d59129425093d9ec805009e75e' }

This give me [ValidationError: child "clientId" fails because ["clientId" is required]]. Changing clientId to client_id in my object works fine, but rename() shouldn't delete my keys if the rename isn't necessary, right? I think this behavior is comming from this commit but maybe it's on purpose and I missed something.

Is that a bug or a feature ?

@AdriVanHoudt
Copy link
Contributor

To me that looks like unexpected behaviour. The override option will delete keys if the rename is not specified. As you can see in the test on that commit it removes key a from the object because it doesn't have an override. Try setting override to false and see if that works.

@JbIPS
Copy link
Author

JbIPS commented Mar 25, 2015

Removing {override: true} gives me [ValidationError: "value" cannot rename child "client_id" because override is disabled and target "clientId" exists], that's why I tried with override

@AdriVanHoudt
Copy link
Contributor

But why does it want rename client_id while it doesn't exist? Weird

@JbIPS
Copy link
Author

JbIPS commented Mar 25, 2015

That's what I'm thinking. I use rename() to ensure all my objects have those keys in camel-case, but some already have, and it's a mess if the good ones are removed instead of just doing nothing.
I tried replacing delete target[item.to]; by continue; and it works perfectly, but this delete must be here for a reason, but why removing target[item.to]?

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

This changed since #593. We'd likely need a new flag to support your use case.

@JbIPS
Copy link
Author

JbIPS commented Mar 25, 2015

Do you have any ideas for the name of the flag? Like ifNecessary or something? I'll try to make a PR asap

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

More something like ignoreMissing. Don't forget that current dev branch is 6.1.0.

@AdriVanHoudt
Copy link
Contributor

Wouldn't this be expected behaviour? That when using rename without override it only renames if the keys exists and otherwise don't do anything?

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

It's been like that for ages, don't act all surprised :)

@Marsup Marsup added the request label Mar 25, 2015
@Marsup Marsup added this to the 6.1.0 milestone Mar 25, 2015
@Marsup Marsup self-assigned this Mar 25, 2015
@AdriVanHoudt
Copy link
Contributor

I've never used that feature so it's just what I expect it to do from reading the docs and this issue so that's why I'm surprised :P

@JbIPS
Copy link
Author

JbIPS commented Mar 25, 2015

Actually, I agree with @AdriVanHoudt. Moreover, it wouldn't need a flag: if the from key doesn't exist, don't do anything. But maybe I'm unaware of use cases that violate this behaviour, I'm still pretty new to Hapi/Joi

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

Seems logical to me, you ask to rename a into b, whatever's into a gets into b.

@AdriVanHoudt
Copy link
Contributor

Ah ok so if a is undefined b will be set as undefined? Ok now I get it. So a flag would make sense here to only rename if not undefined(or null?).

@JbIPS
Copy link
Author

JbIPS commented Mar 25, 2015

So ignoreMissing would mean "If a is missing (= undefined), don't rename"? So maybe ignoreUndefined?

@AdriVanHoudt
Copy link
Contributor

ignoreUndefined is better I think because ignoreMissing might also include null

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

I thought of missing as in "missing key", keys are rarely explicitly undefined, but yours works as well.

@AdriVanHoudt
Copy link
Contributor

A missing key will return undefined so wouldn't that be the same?

@Marsup
Copy link
Collaborator

Marsup commented Mar 25, 2015

Exactly, missing or explicitly undefined is the same to me.

@AdriVanHoudt
Copy link
Contributor

👍

@Marsup Marsup closed this as completed Mar 26, 2015
@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

4 participants