Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DateTime.toSeconds returns seconds.milliseconds #565

Closed
ThorstenBux opened this issue Aug 31, 2019 · 12 comments
Closed

DateTime.toSeconds returns seconds.milliseconds #565

ThorstenBux opened this issue Aug 31, 2019 · 12 comments

Comments

@ThorstenBux
Copy link

Hi,

I'm wondering why DateTime.toSeconds() does return a floating-point number xxxxx.yyy where yyy represents the milliseconds. If I wanted to have milliseconds I would have called .toMilliSeconds()

Expected:
DateTime.fromISO("2019-08-31T00:27:39.689Z").toSeconds() // --> 1567211259

Actual:
DateTime.fromISO("2019-08-31T00:27:39.689Z").toSeconds() // --> 1567211259.689

@icambron
Copy link
Member

The reason people typically want toSeconds is that they want to pass the result a UNIX system that expects datetimes expressed as epoch seconds, and I believe those systems generally allow for decimals in the input. Thus, leaving in the milliseconds makes the conversion to that representation lossless.

You can alway call Math.floor() or Math.round() on it.

@ThorstenBux
Copy link
Author

@icambron, thanks for clarification.

@krukru
Copy link

krukru commented Mar 3, 2020

Hey @icambron, just found this issue after spending some considerable time debugging why my app doesn't work, because I would never expect toSeconds() to return seconds.miliseconds.
I think toSeconds should return exactly what the name suggests, and allow for a different method to return both seconds and milis.
Even better would be to explicitly return the result as an object, with properties seconds and miliseconds. Just return seconds, because it is very explicit about the expected return value/type

@vvo
Copy link

vvo commented Apr 27, 2020

This was also a surprise to me

@carlholloway
Copy link

Likewise. I think at the very least the docs should be explicit that a decimal number is returned

@toddbluhm
Copy link

I just ran into this issue as well. I definitely think it should return just the seconds stripping the millis.

While I totally agree that toSeconds is primarily used for Unix systems (that's what I use it for too), the method is called toSeconds not toUnix. I would be completely happy with this behavior in a toUnix method, but toSeconds is pretty explicit about what it should return.

I would not change this to return an object as that is even less intuitive than the current behavior. toSeconds screams number, not object.

One other possibility is to have an option to enable milliseconds. Something like includeMillis in the options object.

Lastly, whether any changes are made, the current behavior should definitely be added/clarified in the docs 😄

@joepie91
Copy link

I think it's quite reasonable to return a fractional number here, and I don't think it has anything to do with UNIX timestamps. It's fundamentally just a unit conversion like any other, and those can be fractional; if you were to convert "1300 meters" to kilometers, you would also expect to get "1.3 km" and not "1 km".

This is different from extracting the amount of seconds, eg. 49 from 12:20:49, where it would be reasonable to expect only whole numbers; but that's not what this method does, it does a conversion.

From a practical perspective, it also makes sense; while you can round a fractional number, you cannot un-round a whole number. So returning just the whole seconds would make it impossible to get the fractional seconds from that.

That having been said: yes, this should be explicitly documented.

@arkabase
Copy link

arkabase commented Apr 7, 2021

I think you should update the example in the manual :

https://moment.github.io/luxon/docs/manual/formatting.html
dt.toSeconds(); //=> 1492702320 should be dt.toSeconds(); //=> 1492702320.000

@unuseless
Copy link

While some unix systems accepts floating point values for seconds, I believe that is not in specified (see e.g. usage in JWT https://tools.ietf.org/html/rfc7519#section-2 ).
I came across this when constructing JWT's.
From the docs I expected to get back whole seconds.

ebisbe added a commit to ebisbe/luxon that referenced this issue May 20, 2021
As explained on moment#565 .toSeconds() returns seconds.milliseconds to mimic UNIX system behaviour.
icambron pushed a commit that referenced this issue May 20, 2021
As explained on #565 .toSeconds() returns seconds.milliseconds to mimic UNIX system behaviour.
@Abhinavmod
Copy link

LuxonDateTime.fromSeconds(1642832200,{zone: "America/Chicago"}); returns an offset of 1080 not sure how to get this resolved works fine for other date and time

@adamreisnz
Copy link

adamreisnz commented Mar 28, 2022

Also ran into this issue, and it in fact created a bug, because the systems we convert this to seconds for, do not accept floating point numbers. So I'm not sure what this assumption was based on, but I think the majority of users would expect the output of toSeconds() to be an integer.

@icambron is there any chance this can be revisisted in the future?

Having to wrap every occurrence of toSeconds() with a Math.floor() or Math.round() is pretty annoying, especially since moment had a simple to unix conversion which would just return seconds only.

@cgibson-swyftx
Copy link

One issue that I've found with this is that TypeORM rounds the number automatically when inserting it into MySQL. So while the DB looks correct, I had no idea that the actual value was a decimal and was messing up my API calls as they were converting to a string which included the decimal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests