There's a few places in the code where Date types are expected, e.g. https://github.com/gpbl/react-day-picker/blob/master/src/DateUtils.js#L33 which is by design, but I think there is some value in allowing Dates passed as strings which are then coerced to Date types when necessary.
Depending on the use-case, we don't always have our values as Date types and it's a bit tedious to have to make sure Date types are passed, for example. to the addDayToRange util. What are the thoughts around allowing dates strings to be passed which are then converted to Date types (if validated)? An error could be logged if the date string is not valid.
Having dates passed as strings would be also a solution for https://github.com/gpbl/react-day-picker/issues/130 as well. I'm very open to implement this feature, however I'm tempted to accept dates only in the form of YYYY-MM-DD. What do you think?
Mmh, I can see what that would be simple and predictable, but it still means handling formatting every date that is passed to Day Picker. What about passing a string or a date and then the library checks the type and, if it's a string, simply wraps it with new Date(...)? The validity of it could be checked with moment.isValid as well and a warning output to the console if invalid.
Thanks @willmcclellan for the feedback, I'm still thinking about it... the reasoning behind native Date objects was to keep this kind of problems out of the component, and let the implementer choose how to deal with them. Now you do:
addDayToRange(
new Date(dayAsString),
range: { from: new Date(fromAsString), to: new Date(toAsString) }
)
then you will do:
addDayToRange(
dayAsString,
range: { from: fromAsString, to: toAsString }
)
Is the first option really so much more verbose? Could you provide an example of your actual use and how would you rather see it implemented?
@gpbl It's definitely a convenience thing and the problem arose from me while using seamless-immutable, which strips some of the prototype methods from Date objects (causing things like date.getTime() to fail.
Couple of the (minor) issues I experienced:
fromAsString/toAsString are undefined, then I don't want a value to be set, which means I have to use a ternary for setting from/to.range object to the utils.This is something that we have to repeat around our app as well which is why I was thinking the library handling all this would be quite useful. But yeh, it's all about convenience and would understand not wanting to introduce bloat.
which strips some of the prototype methods from Date objects (causing things like date.getTime() to fail
Would using moment.js date objects here help? I think this is a much more common case. Something like <DayPicker library={ moment } />. In this case, it would be easier to parse strings.
Please do not add moment as a hard dependency! Moment is great - but huge!
Maybe it is possible to detect if the passed dates are moment objects and in case use them for comparison. This way moment won't be a dependency.
As dates as strings are often source of problems, especially when dealing with timezones, I wouldn't support them in the component. As this issue is pretty old, I'll close it. Let see if the problem rises for other people as well before considering this feature.
Thanks for your understanding @willmcclellan !
Most helpful comment
Please do not add moment as a hard dependency! Moment is great - but huge!