Explicitly label unix timestamp as a number

[Slurpgoose]

I missed "prevent errors, you should cast the numeric type of the time to UTCTimestamp" when reading this initally. This extra comment may help future library users avoid accidentally cast timestamp as a string and open new issues.

Type of PR:

PR checklist:

• Addresses an existing issue: fixes #
• Includes tests
• Documentation update

Overview of change:

Is there anything you'd like reviewers to focus on?

[timocov]

I'm afraid that they shouldn't be UTC actually. It's preferred to provide them in UTC, but to emulate "timezones", you might to pass date with offset (we'll provide examples/docs for that in #497). Thus, I think that it's better to write about that in a fix for that issue to avoid confusing and more misunderstanding.

[timocov]

I mean, the nature of timestamp is UTC-based by-default (it's number of seconds since 01-01-1970 00:00:00 UTC), what else there could be? But to emulating timezones you can use some tricks, but it isn't default.

[Slurpgoose]

I apologize if the intention of the commit was unclear, I had an issue with the error message stating "string format must be in yyyy-mm-dd format" when a Unix timestamp is passed as a string.

I spent an extra 20 minutes looking through the docs to see if there was a configuration setting that had to be passed to accept Unix based timestamps.

It was not until I found issue #414. Before I found my stupid mistake. I agree that it is a mistake on my end because epoch timestamps should not be stored as strings. Unfortunately the error provided by the library made it difficult for me to understand what specifically was incorrect. I thought that by adding the one line to the documentation it would a good hint for library users in the future to quickly check their data type first.

I understand your concerns with the UTC label as it may mislead people to think that the library will localize timestamps by default.

thank you for the reference to the other issue as it is something that I have not considered either. As well thanks for maintaining this great library :)

[timocov]

I had an issue with the error message stating "string format must be in yyyy-mm-dd format" when a Unix timestamp is passed as a string.

I guess we'll help solve such issues faster in #315. But probably we should say that it should be number - 123 (not string-like-number value '123') then in this PR?

Unfortunately the error provided by the library made it difficult for me to understand what specifically was incorrect

Agree. Do you have full error message you got here? I thought that we log passed date value as well in the message?

I thought that by adding the one line to the documentation it would a good hint for library users in the future to quickly check their data type first.

Yes, sure! But I think that it shouldn't be about whether that value should be UTC or not, that note should be about type of the value IMO (string vs number).

[Slurpgoose]

But probably we should say that it should be number - 123 (not string-like-number value '123') then in this PR?

I completely agree, I think that makes the most sense. I attempted to copy the format from Business day object and just missed the mark.

Do you have full error message you got here?

Error: Invalid date string=1595284200, expected format=yyyy-mm-dd

you are correct, the issue is simply user error and I have already put way more thought into it than its worth.

If this issue happens frequently maybe adding a second condition isNan(parseInt(timestamp) to thow an error message specifically for a number passed as a string value at line src/api/data-layer.ts Line 84 would eliminate all confusion.

Although after looking at the code and running the example it seems pointless to cover an edge case that the new Date constructor does not even handle.

Cheers, sorry for taking up your time.

[timocov]

If this issue happens frequently maybe adding a second condition isNan(parseInt(timestamp) to thow an error message specifically for a number passed as a string value at line src/api/data-layer.ts Line 84 would eliminate all confusion.

Not sure what exactly condition we need to add there, because parseInt will handle both of strings properly:

But I think it's a good direction to handle it in #315 anyway, thanks!