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

Result is incorrect #42

Open
alexby opened this issue Jul 31, 2022 · 6 comments
Open

Result is incorrect #42

alexby opened this issue Jul 31, 2022 · 6 comments

Comments

@alexby
Copy link

alexby commented Jul 31, 2022

Context

  • node version: 16.13.0
  • module version with issue: 2.0.1
  • last module version without issue:
  • environment (e.g. node, browser, native): node
  • used with (e.g. hapi application, another framework, standalone, ...): standalone
  • any other relevant information:

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

const dateTime = "2012-03-29T12:00:00-06:00";
const schema = Joi.date().format("YYYY-MM-DD[T]HH:mm:ssZ");
const result = schema.validate(dateTime);
console.log({
  value: result.value,
  type: typeof result.value,
  instanceDate: result.value instanceof Date,
});

What was the result you got?

{
  value: 2012-03-29T18:00:00.000Z,
  type: 'object',
  instanceDate: true
}

What result did you expect?

{
  value: '2012-03-29T12:00:00-06:00',
  type: 'string',
  instanceDate: false
}

So I see 2 issues here:

  1. The type is changed (that could be a feature, not a bug);
  2. (that is more important) The time shift is changed;
@Marsup
Copy link
Collaborator

Marsup commented Aug 2, 2022

  1. It is indeed a feature, you ask for a date validation, you get a date.
  2. The date you see depends on the timezone you've set on your machine. This module does nothing more than this, if the result is wrong, then it's very likely a moment issue.

@kanongil
Copy link
Contributor

kanongil commented Aug 2, 2022

Regarding 2: The Date object does not store any timezone information, so it will always use the zero offset (Z) when asked to convert to an ISO string representation. As such, the behaviour is as intended.

@alexby
Copy link
Author

alexby commented Aug 3, 2022

From my point of view (a user, who uses the library from time to time, but not on everyday basis) this behavior is not obvious, is it a good practice for joi to change the type?
The Date cannot give us the correct result. And yes, the time in another timezone is incorrect (even if it's the same unix time), the offset is a part of the datetime payload, and the library is changing this payload while returning the result.
I see moment in the dependencies list, and it can handle the offset correctly. Why the result is not a moment object?

@kanongil
Copy link
Contributor

kanongil commented Aug 3, 2022

It is standard behaviour for Joi to change the type, unless the convert option is set to false.

This specific module extends the base Joi.date() type. If you feel it is not enough, someone needs to create a completely custom Joi.moment() or whatever type with whatever logic that makes sense.

@Marsup
Copy link
Collaborator

Marsup commented Aug 3, 2022

Joi schemas, unless instructed otherwise, describe the types you get in the end, not the types you receive. Moment is used for parsing, that's it, the contract to get a date still stands. This behavior is mentioned on the very 1st sentence of the date type documentation, I'm not sure we can do much more than that for casual users such as yourself if you don't read the docs 🤷🏻‍♂️

If you want more granular control on those fields, you can also use https://joi.dev/api/?v=17.6.0#anyrawenabled, which will both validate and give you the original string. You can also create your extension to get moment objects as suggested by Gil if that's what you want.

To me, this representation is as good as any, as a date or a string won't give you correct results either unless you have the location.

@philip-nicholls
Copy link

A Date object is not a string. The fact that the validation does not fail in this case means this library is validating a "date string". Perhaps this module should be renamed to Joi.dateString() or documentation could be made clearer.

For anyone looking for actual Date validation, I have found that this seems to work:

const JoiDate = Joi.object().instance(Date);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants