hapijs / joi-date

Joi extensions for dates
Other
83 stars 25 forks source link

Result is incorrect #42

Open alexby opened 1 year ago

alexby commented 1 year ago

Context

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 commented 1 year ago
  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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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);