Skip to content

Conversation

@Cmdv
Copy link
Contributor

@Cmdv Cmdv commented Sep 22, 2020

What does this pull request do?

add function getTimezoneOffset and fixes #8

Where should the reviewer start?

src/Effect/Now.js

How should this be manually tested?

Other Notes:

Being this is purescript-now I used:

exports.getTimezoneOffset = function () {
  var n = new Date(Date.now());
  return n.getTimezoneOffset();
};

repl output from UK based local:

> getTimezoneOffset
(Minute -60)

Fell like if we'd want Date -> Effect Minute it might need to live in purescript-datetime?

@garyb
Copy link
Member

garyb commented Sep 22, 2020

Date -> Effect Minute wouldn't make sense/be possible in purescript-datetime since that representation doesn't have time zones. There's a function like that in purescript-js-date though. 🙂

@Cmdv
Copy link
Contributor Author

Cmdv commented Sep 22, 2020

Date -> Effect Minute wouldn't make sense/be possible in purescript-datetime since that representation doesn't have time zones. There's a function like that in purescript-js-date though. 🙂

I spotted that implementation when looking for something similar, in purescript-js-date you can pass around JSDate because it's foreign import data JSDate :: Type which I see isn't the equivalent of Date now I've looked at it 🤦

@garyb
Copy link
Member

garyb commented Sep 22, 2020

Yeah, the idea with the purescript-datetime and purescript-now libraries is they're not JS specific, and provide a basic mechanism for dealing with date/time stuff as a common basis. Originally I had a type in datetime that allowed you to attach a Minute offset to a Date / Time / DateTime, but it wasn't really doing anything different than a Tuple provides, so we removed it to avoid giving the impression it was actually doing something clever or useful. 😄

datetime should actually be completely non-FFI, but I got lazy when implementing it and used a JS date type in the FFI to deal with logic when modifying date/times and such, to avoid having to implement all the rules around that. I still hope one day I or someone else will write it fully in purescript.

now is separate from js-date and minimal because I wanted the interface to be very small to make it easy for other FFI implementations (native, erlang, whatever) to make this library work.

@garyb
Copy link
Member

garyb commented Sep 22, 2020

Sorry, one more change: the duration type is called Minutes rather than Minute.

@Cmdv
Copy link
Contributor Author

Cmdv commented Sep 22, 2020

datetime should actually be completely non-FFI, but I got lazy when implementing it and used a JS date type in the FFI to deal with logic when modifying date/times and such, to avoid having to implement all the rules around that. I still hope one day I or someone else will write it fully in purescript.

interesting that might be a nice challenge, going through prolonged down time so trying to keep busy 😄

Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll let @thomashoneyman merge this, as I'm not sure what we're doing with changelog and stuff with the library as it is just now!

@Cmdv
Copy link
Contributor Author

Cmdv commented Sep 22, 2020

Thanks @garyb, I might pose some questions on purescript-datetime as to what non FFI version might look like, to get me rolling.

@thomashoneyman
Copy link
Contributor

Went ahead and merged — we’ll add this to the changelog when we get to this library’s updates :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Include function to fetch timezone offset for certain date

3 participants