DateTime.toISO8601 truncates to seconds

[witoldsz]

Hi, I have a problem with incorrect dates generated in my forms and I have found the problem is in toISO8601 function, it skips the milliseconds part of the date.
See this: https://runelm.io/c/3cr

Time.DateTime.fromISO8601 "2017-04-22T19:05:31.040Z"
    |> Result.map Time.DateTime.toISO8601
    |> Result.withDefault ""

 input: 2017-04-22T19:05:31.040Z

output: 2017-04-22T19:05:31Z

[oldfartdeveloper]

@witoldsz thanks for submitting. Would you like to submit a fix? Otherwise I'll look at it this evening.

[witoldsz]

I could try, but I am not sure if I'll get time for this today. Maybe in the coming week? Let's do this: whoever get into this first, let that person announce it here :)

[witoldsz]

OK, so I have started looking at it just now. Fix is a one minute task, the tests are to be fixed and one or two cases should be added. Will issue a pull req soon.

[oldfartdeveloper]

@witoldsz It dawned on me to look up ISO8601; I'm making sure there's a standard for expressing milliseconds.

Haskell, for example, has formatISO8601 for generating to seconds resolution and has the following for more granular resolution:

• formatISO8601Millis
• formatISO8601Micros
• formatISO8601Javascript (alias for formatISO8601Millis; see discussion below)

etc.

What's interesting is that they (Haskell) follow Mozilla standards which stops at the millisecond level but they caution that not all other browsers might be compatible.

I like Haskell's compromise:

• The "standard" formatter limits to the seconds resolution.
• The "millisecond" formatter limits to the milliseconds resolution.

In Elm's case, this would translate to:

• toISO8601
• toISO8601Millis

I like that solution.

Thoughts?

[witoldsz]

I think it's looking for making a simple thing complicated. What would be the real use case of truncating ISO8601 date to seconds? For me it's like placing a mine, so inattentive developer can step on it and BUM, we have a new bug (hard to spot, actually).

I can't find a real case for this, really.

[zwilias]

Millisecond resolution seems like a sane default; though I'm not sure about introducing that as a patch-level change.

Haskell's compromise makes sense, but in the interest of interop with the platform, I think it makes sense for a function in Elm to have the JS-like variant as a default, and a truncating option as an alternative, rather than the other way around.

I think our context of running in the browser comes with certain expectations, and following the principle of least astonishment; a function that at face value looks like it would do the same as toISOString, probably should do the same.

[oldfartdeveloper]

I suspect that most users are not interested in seeing the milliseconds most of the time. And different programming languages handle it differently.

The biggest reason I can think of for adding the toISO8601Millis method instead of modifying the existing one is that the existing behavior doesn't change; just a new capability is being added.

The new capability is, as @zwilias notes, a patch-level change, whereas modifying what toISO8601 does will have to at least, imho, be a minor-level change.

Changing existing behavior violates the principle of least astonishment.

[oldfartdeveloper]

But I see the attraction of complying w/ https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString

There are pros and cons either way we go.

[witoldsz]

I would ask my question again: what is a possible use case for truncating millis from ISO8601 representation? I do understand truncating the date to seconds, or even minutes when showing it on the form, but who actually displays ISO8601 in the UI for end users? I do truncate to seconds when displaying time almost always, but when creating an HTTP request, I always switch to UTC/ISO.

[oldfartdeveloper]

@witoldsz good point. Another point in your favor is that toISO8601 presently truncates the millisecond fraction; that leaves an ambiguity that I don't like. Such as why doesn't it round instead?

[oldfartdeveloper]

So I agree that the existing behavior is "not right", and that the best way to fix it is to extend the output to include the milliseconds. Opinions on whether this is a patch or minor level change?

[witoldsz]

I would call it a patch, but that is just me. The docs did not mention anything about rounding and truncating, so for me it's just a fix of a bug (this would actually fix my app, but yesterday I have rewritten it, so I do store the original ISO in the model as String; once this gets merged I could switch back to DateTime in my model).

[oldfartdeveloper]

Thanks, I agree that a patch is a better solution also.

[oldfartdeveloper]

Okay, I've developed the test for it and it mimics your failure. What do you have ready?

[witoldsz]

Okay, I've developed the test for it and it mimics your failure. What do you have ready?

What do you mean? The pull request #31 includes test cases…

[oldfartdeveloper]

Ahh, I hadn't seen the PR yet. Okay, I'll process it now.

[oldfartdeveloper]

Merged. I'll do a bit of testing and package it in few minutes

[witoldsz]

I have just spotted an example of toISO8601 in the /README.md, it should be fixed as well.

[oldfartdeveloper]

Just published (sorry, didn't see your comment above.). Thanks, I'll fix that. In the meantime, you have your elm-time fix in. Thanks for your fix and its tests.

[witoldsz]

Thanks for your time too, it's Sunday actually :)

P.S.
I can see the new release 1.0.4, but /elm-package.json has "version": "1.0.3". Is that OK for Elm package repository?

[oldfartdeveloper]

Yeah, I just updated it. The procedures are not documented "airtight". I'll fix when I submit the fix for the README.md (it will be 1.0.5).

On another note, I saw 3 places in the README.md that need to be fixed; does that match w/ what you saw?

[oldfartdeveloper]

Oh, and I don't think the version in the elm-time elm-package.json makes a difference for users.

[oldfartdeveloper]

Updated README and published; version is now 1.0.5.