fp-ts / optic

A porting of zio-optics to TypeScript
https://fp-ts.github.io/optic/
MIT License
115 stars 4 forks source link

feat: added Date optics #24

Closed jessekelly881 closed 1 year ago

jessekelly881 commented 1 year ago

Added data/Date with Lenses for year, month, day, hour, minute, second, and millisecond.

gcanti commented 1 year ago

What's the meaning of the utc parameter? Intuitively looking at just the signature, I would have thought "use UTC", but from the code it doesn't seem to be the case.

jessekelly881 commented 1 year ago

Most of the js Date functions have UTC equivalents that (I believe) set the hour, day, etc. as if it were that hour/day UTC time and then converts the result back to local time. If the date is already in UTC time, calling setHours(2) will just update the time to 2am. But if the date is UTC - 1 for example it will set the hour to 1am(local time). I will update the docs and tests to make it a bit clearer what's going on. https://stackoverflow.com/questions/32647928/setutchours-adds-an-hour-to-time-passed-to-the-function

IMax153 commented 1 year ago

@jessekelly881 I'm not sure that the proposed API is necessarily desirable. I would almost argue that we should just treat every date the same and avoid calling the setUTC* variants internally. If the user want to use UTC, then their input Date to the optic should be in UTC.

jessekelly881 commented 1 year ago

Ok. I'm inclined to agree. I'll update the pr. Do we prefer Date.hour() or Date.hour?

IMax153 commented 1 year ago

I'll leave that question for @gcanti 😃

gcanti commented 1 year ago

Do we prefer Date.hour() or Date.hour?

hour

jessekelly881 commented 1 year ago

Cool! I updated it.

gcanti commented 1 year ago

Thanks @jessekelly881 @IMax153