Extract parseISO from toDate and disallow passing strings into functions

[kossnocorp]

See #1015 for the background.

toDate is the function that converts passed argument to a date object and used virtually in every function in the library. The thing is that it weights 1.27 KB (after heavy size optimization), so the minimal build size couldn't go below the number.

For example, addDays size is 1.31 KB (it was 983 B in v1).

The size bottleneck is in ISO 8601 string parsing code, so even if you don't use strings as arguments, you're going to pay the price.

The solution to the problem is to extract this code into parseISO and make date-fns functions accept only dates and numbers as arguments. It will make the library slightly more difficult to use for those who use strings to store dates but more lightweight for others.

This is a breaking change and will cause a backlash, but I believe it's worth it. Many choose date-fns because it's light and without this change, we won't be able to achieve the best possible result.

[leshakoss]

I agree. A lot of people choose date-fns because it is lightweight and we should try to be the best library possible for this type of users — while maintaining the same flexibility and cover all possible date use cases. Our main intent was always to work with Date objects, and while many users enjoyed that we treat ISO-8601 strings as first class citizens as well, I believe isolating the parsing code in its own separate function is the best solution because:

1. You only need to parse a date once, and invocation of parser code in every function is overkill. I feel like every function should only be doing one thing.
2. Working with Date objects is our primary use case and many users just don't need to work with ISO-8601 string dates.
3. [debatable] parseISO being a separate function forces users to describe their data flow more explicitly, which could reduce the number of bugs caused by the type mix up (why doesn't date.getTime() work here? Oh, because I haven't converted the string to date yet).

[leshakoss]

And BTW I think that this change would mean that most of the functions wouldn't need options argument, so we won't need to generate fp/{functionName}WithOptions for them anymore, which would make FP module much easier to maintain

[stof]

Shouldn't this issue be closed as #1023 was merged ?

[marnusw]

I've been thinking about this change quite a bit because, to be honest, for me it has been a blocker upgrading many of my projects from alpha.26 to alpha.27. My concern is not so much that arbitrary date strings (e.g. partial ISO dates) are no longer supported, but rather that the fully formed ISO date output from JSON.stringify(new Date()) can no longer be used.

It was awfully convenient to know one could read a Date from a database on the server, JSON.stringify as part of a larger payload and send it to the client. Then on the client the payload can just be used as needed and the date passed into date-fns without regard for whether it has been converted to a Date or left as a string, and dates could similarly just be sent back to the server as JSON. In short, all the date handling/conversion was entirely transparent.

I'm completely on-board with taking the bulk of the string parsing out of toDate because the appeal of date-fns certainly is to include only minimal functions as needed. I also do think the trade-off of supporting only fully formed ISO dates might add a lot of benefit without too much cost. I'm thinking something as simple as:

if (typeof dirtyDate === 'string' && dirtyDate.length === 24) {
  let parts = dirtyDate.match(/(\d{4})-(\d{2})-(\d{2})T(\d{2}):(\d{2}):(\d{2})\.(\d{3})Z/)
  if (!parts) {
    throw new Error('...')
  }
  return new Date(+parts[1], parts[2] - 1, +parts[3], +parts[4], +parts[5], +parts[6], +parts[7])
}

There may be edge cases I'm not aware of or other complications but I thought it worth asking your thoughts?

[kossnocorp]

@marnusw I feel your pain. I want to keep date-fns as reliable and lightweight as possible but it shouldn't be too painful. After a while, I see why this change might be painful, so I find your idea appealing. I need to think a bit about what you just said, however here's what I don't like about it:

• JS never throw exceptions if arguments are incorrect because you don't want your application break when a user enters nonsense.
• Given that, we will have to return Invalid Date and because of that accepting only fully formed will lead to bugs.

I see the problem and I'd like to find a solution to it, but I'm not certain that's the one.

[leshakoss]

Was fixed in v2.x.x!