milliseconds parsed incorrectly in deserializeTime #1766

Activity

1. davidboweninrupt

   added

   bugSomething isn't workingSomething isn't working

   on Nov 21, 2022
2. davidboweninrupt commented on Nov 21, 2022

   

   davidboweninrupt

   on Nov 21, 2022

   Author

   More actions

   I worked on a fix for this by having Time.millisecond being non-integer. However, that caused me to need to change serializeTime too.

   I started to look around to enhance the tests but struggled because all the tests I was running were in the same unit so I had to keep fixing them one at a time as each run stopped on the first error.

   As a result I ran out of time to work on this problem.

   I was tempted to do a "quick fix" by having deserializeTime truncate the string to be the first three characters. That would stop the large-scale problems, but would still give incorrect results because it would always be the floor, not the rounded-off value when there were more than three digits. It would also not fix the problem when there were less than three digits.

   Rather than submit a bad fix I decided to drop my code and create this issue instead.
3. Vinnl commented on Nov 21, 2022

   

   Vinnl

   on Nov 21, 2022

   Contributor

   More actions

   Oh nice, that explains the root of the issue. In that case, I'd expect the fix to be something like this at

   solid-client-js/src/datatypes.ts

   Lines 255 to 257 in 9314ead

   	const utcMilliseconds = optionalMillisecondString
   	? Number.parseInt(optionalMillisecondString, 10)
   	: 0;

     const utcMilliseconds = optionalMillisecondString
   -    ? Number.parseInt(optionalMillisecondString, 10)
   +    ? Number.parseInt(`0.${optionalMillisecondString}`, 10) * 1000
       : 0;

   👍React with 👍1Reacted by David Bowen
4. ThisIsMissEm

   added

   TriagedThis means that we've a ticket to look at this in the futureThis means that we've a ticket to look at this in the future

   on Nov 25, 2022