Since JavaScript has no support for nanoseconds in Date, I'd vote for dropping the nanoseconds. Currently the reader just blows up when reading a String with java.sql.Timestamp printed in CLJ, since that is printed in nanosecond precision.

While probably not the best solution, I attached a patch for cljs.reader/parse-and-validate-timestamp fn that just drops the nanoseconds from the millisecond portion. Would be easier to just strip the ms string to 3 digits but that would cause "2012-12-30T23:20:05.066980000012312312123121-00:00" to validate.

Thomas Heller
added a comment - 05/Feb/13 3:28 PM Since JavaScript has no support for nanoseconds in Date, I'd vote for dropping the nanoseconds. Currently the reader just blows up when reading a String with java.sql.Timestamp printed in CLJ, since that is printed in nanosecond precision.
While probably not the best solution, I attached a patch for cljs.reader/parse-and-validate-timestamp fn that just drops the nanoseconds from the millisecond portion. Would be easier to just strip the ms string to 3 digits but that would cause "2012-12-30T23:20:05.066980000012312312123121-00:00" to validate.

One could extend the js/Date prototype with a setNanos/getNanos method and call them accordingly. I'd offer to implement that but the cljs.reader/parse-and-validate-timestamp function scares me, any objections to rewriting that?

Not sure if its a good idea though, messing with otherwise native code might not be "stable".

Not convinced that keeping the nanos around is "required" since javascript cannot construct Dates with nanos, but its probably better not to lose the nanos in case of round tripping from clj -> cljs -> clj.

We definitely don't want to mutate Date's prototype without namespacing. I'm not sure we want to mutate Date's prototype at all. That's why I suggested a ClojureScript type with the same interface as Date just as Google has done in Closure.

David Nolen
added a comment - 21/Feb/13 12:25 AM - edited We definitely don't want to mutate Date's prototype without namespacing. I'm not sure we want to mutate Date's prototype at all. That's why I suggested a ClojureScript type with the same interface as Date just as Google has done in Closure.

This patch (CLJS-454.diff) takes the "truncate to millisecs" approach which I think is correct considering that the clojure reader does the same thing (but they truncate to nanoseconds instead).

The patch does some refactoring of parse-and-validate-timestamp and in the process fixes CLJS-564 as well as a subtle bug where the interpretation of the fraction part differed between clojurescript and clojure (see commit msg for details)

Jonas Enlund
added a comment - 31/Jul/13 4:21 AM This patch (CLJS-454.diff) takes the "truncate to millisecs" approach which I think is correct considering that the clojure reader does the same thing (but they truncate to nanoseconds instead).
The patch does some refactoring of parse-and-validate-timestamp and in the process fixes CLJS-564 as well as a subtle bug where the interpretation of the fraction part differed between clojurescript and clojure (see commit msg for details)
I also added tests.

There are duplicate tests in core-tests[1] and reader-tests[2]. This wouldn't be a big deal but each of them generates 7000+ assertions which is a bit excessive. Also note that the assertions in reader-tests doesn't do any padding so instant literals of the form #inst "2010-1-1..." are generated (which are not valid but goes unnoticed in the current implementation). This is why the CLJS-454.diff patch fails to pass the tests.

Should I update the patch where I remove some of the tests (either in core-tests or reader-tests)?

I note that Clojure includes these tests so probably not a good idea to remove them. Also they actually test different things right? I'd rather see the tests fixed if they need to be adjusted for the new behavior.

David Nolen
added a comment - 31/Jul/13 4:59 AM I note that Clojure includes these tests so probably not a good idea to remove them. Also they actually test different things right? I'd rather see the tests fixed if they need to be adjusted for the new behavior.