ember-changeset-validations
ember-changeset-validations copied to clipboard
Date validator request
This package is currently not providing a Date validator, which would be very useful for validating date-type inputs. Such a validator is already provided by ember-validators but is not in this project.
https://github.com/offirgolan/ember-validators/blob/v2.0.0/addon/date.js
Having it available would allow us to use changesets for date inputs with something better than
validateFormat({ regex: /[0-9][0-9][0-9][0-9]-[0-9][0-9]-[0-9][0-9]/ })
which is currently the best, but somewhat hacky, implementation option.
I think this would be a great idea! Would happily accept a PR!
@snewcomer Done, let me know if it works for you.
@snewcomer Thanks a lot for implementing the date validator in #218. Can't count how often I have implemented something like this in app space. I should have pushed one of them upstream. :man_facepalming:
I'm wondering why you decided to not support strings. I often used a date validator directly together with <input type="date"> and other date input types. This would mean to support a limited range of strings:
yyyy-MM-dd(date)yyyy-MM-ddThh:mmwith optional seconds and milliseconds (datetime-local)yyyy-Www(week)yyyy-MM(month)yyyy(year)
Would you accept a pull request implementing such a support?
👋 Good question. So I thought it might be good to not allow strings for 2 reasons.
- Creating dates with a string is a source of error if the string doesn't contain time information.
- If you are using a library like
moment, the dates could be constructed as Moment and passed in as arguments (assumingmoment('02-01-2019') <= moment('01-01-2019')works...does it?)
What are your thoughts? Is the current validator somewhat flexible enough for your use case?
Creating dates with a string is a source of error if the string doesn't contain time information.
You could also see it the other way around: Information about the date is lost on conversion from ISO8601 string to a Date object. As far as I know you can't tell from the date object if
- it included a timezone or was in locale time (
yyyy-mm-ddThhThh:mm:ss+00:00vsyyyy-mm-ddThhThh:mm:ss) - included time information (
yyyy-mm-ddThh:mm:ss) or way day-only (yyyy-mm-ddThh) or for a specific calendar week (yyyy-Www), month (yyyy-mm) or even only about a year (yyyy).
In most cases the date should include time and time zone information but for some dates this conversion could be wrong. E.g. new year is independent of time zone. It was at 2020-01-01T00:00 this year for everyone regardless of the time zone. But the new year was celebrated in New York at 2020-01-01T00:00-05:00.
Sometimes I prefer to not convert the ISO 8601 date strings to not loose that information.
If you are using a library like
moment, the dates could be constructed as Moment and passed in as arguments (assumingmoment('02-01-2019') <= moment('01-01-2019')works...does it?)
Not sure about moment to be honest but for native Date this is working cause it implements valueOf(), which returns the timestamp. So new Date('2010-01-01') < new Date('2010-02-02') is the same as new Date('2010-01-01').valueOf() < new Date('2010-02-02').valueOf(), which is the same as 1262304000000 < 1265068800000.
Is the current validator somewhat flexible enough for your use case?
I think it's flexible enough but I don't like that I need to convert the date string to a Date object only for validation purposes.
I think you have brought up some good points. I would also note...
prefer to not convert the ISO 8601 date strings to not loose that information
Either we convert it to a Date object or we ask you to do so. Somebody has to do it so we can do propert comparisions. I would just prefer that we are library independent. moment, luxon, date-fns, dayjs will help with these normalization issues common to JS apps.
My concerns revolve around the variety of permutations the strings may come in as you have noted. The date from the input might be different string format than the date based to before.
new Date('2020-01-01T10:00:00Z')
new Date('2020-01-01T10:00:00')
Any other thoughts or ideas around supporting strings?
I understand your concerns but to be honest I think most of them are caused by relying on Date objects rather than using ISO8601 strings.
I think this could be implemented safely without introducing performance issues if having three restrictions:
- Validated value and options must use the same type. If the validated value is a
Dateobject, all options must beDateobjects as well. If the value is astringall options must be strings as well. - If the value is a string, it must be an ISO 8601 string that is an allowed value for an input element.
- If the value is an ISO 8601 string, the same format must be used for value as for options.
The first restriction ensures that we don't have to deal with date parsing at all. The second one limits the possible formats to a small set. The third one allows us to do a simple string comparison.
If I didn't missed something the implementation should be straight forward. To verify that value and options are using the same format comparing the length of the strings should be enough. ISO 8601 spec ensures that two dates of the same format could be compared as strings.
Not sure if it should also verify that it's a valid date string. E.g. either throw or fail validation if value is 2020-02-31. I think such a check would require parsing the date to not deal with all that complexity ourselves. But if it's only an assertion or optional feature it would not have a performance impact.
I could be on board with string comparison! And I agree length should be enough (although - or / would give us a false positive). I'm not sure we need to only limit to ISO8601 either (so <input type="date"> works). What do you think? I'd be fine as long as we have some sort of string validator...
I'm not sure we need to only limit to ISO8601 either
Not limiting to a subset of ISO 8601 opens up a wide range of edge cases. The spec allows so much different ways to express the same information. Also I don't see a use case. Consumers either validated the return of a native input field or of a library. In the later case it's much likely that the library either returns a Date object or one of the ISO 8601 formats that are used by web platform. Beside the formats used by native inputs this is only yyyy-mm-ddThh:mm:ss.ssssT, which is used by Date.toISOString().
Not limiting to ISO 8601 would allow formats for which string comparison does not work. E.g. a consumer may use it with German locale format, which is dd.mm.yyyy.
What string formats do you like to support additionally?
Edit: Just noticed that this may be caused by confusing the format returned by Date.toISOString() with the spec itself. That method is only using one format out of a huge set of possibilities. Wikipedia lists some of them: https://en.m.wikipedia.org/wiki/ISO_8601 All formats used by native HTML inputs are specified by ISO 8601 spec.
Ah I assumed the value when mutating an input[date] was not iso8601, but if it is, then 👍 . Would you be willing to put up a PR with a test / dummy example with the native input and we can start from there?
@jelhan ping! Lmk if you would like me to take it on! I got some time this week.
Lmk if you would like me to take it on! I got some time this week.
This would be awesome. I'm totally overloaded with Open Source stuff currently. :cry:
It'd be great to allow the before, onOrBefore, after, onOrAfter options accept a function that returns a date. This way every time the validator is executed we can re-evaluate those values instead of evaluating only when the validation is defined. This is useful if we need to use a value that may change as the value for before, onOrBefore, after, onOrAfter (ex other date properties on the changeset).
I'd be happy to PR this change if there is agreement here.
@steveszc Make sense. But wondering if this is needed on all validators. Or is date validator a special case?
I think if we started with the date validator, that would be a great starting point! Lmk if you have any questions @steveszc
@snewcomer just put up a PR adding the function support for date validator options. #281
Thank you!! 3.13.0
@jelhan Do you have an interest in taking a look at this PR? I did it sort of naively. But the plan would be to use this here if it fit your use case!
https://github.com/offirgolan/ember-validators/pull/100