utrecht icon indicating copy to clipboard operation
utrecht copied to clipboard

Calendar component has an inconsistent API

Open sergei-maertens opened this issue 1 year ago • 0 comments

I've touched on this topic in Slack, and #1747 also points out the weirdness that this can create.

The prop types of the Calendar component (https://nl-design-system.github.io/utrecht/storybook-react/index.html?path=/story/react-calendar--default) is not consistent:

  • currentDate, minDate, maxDate: Date instance
  • events.date: string in ISO format (processed via date-fns/parseISO)
  • onCalendarClick callback argument is an ISO string of the selected date (with time information!)

the (stripped down) interface definition:

interface CalendarProps {
  events?: Array<{
    date: string;
    emphasis?: boolean;
    selected?: boolean;
    disabled?: boolean;
  }>
  currentDate?: Date;
  minDate?: Date;
  maxDate?: Date;
}

This makes it awkward as the user of the component is constantly converting between Date and string types and the abstraction of the calendar itself is leaking.

I think this API should be made consistent - either use ISO 8601 strings or Date instances everywhere, or perhaps accept both Date instances and strings in the correct format.

What's happening now if you provide a Date instance in the events is that parseISO(someDate) returns a new Date instance 'Invalid Date', which is equivalent to silent failure mode.

There is also a reality that events will be returned from APIs outside of "our" control and they may return serialized dates (strings) with any or all of the following:

  • naive dates without timezone information
  • unix timestamps rather than ISO-8601 strings
  • strings with local formats, such as d-m-jjjj

All of the above essentially means that the caller of the Calendar component needs to do parsing/processing anyway of the data/props that are being fed to the component. To avoid endless serialisation/deserialisation I think it'd make a lot of sense to accept Date instances everywhere.

sergei-maertens avatar May 31 '23 14:05 sergei-maertens