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

Flip Zone.offset, rename as Zone.utcOffset #397

Closed
mattjohnsonpint opened this issue Oct 12, 2016 · 9 comments
Closed

Flip Zone.offset, rename as Zone.utcOffset #397

mattjohnsonpint opened this issue Oct 12, 2016 · 9 comments

Comments

@mattjohnsonpint
Copy link
Contributor

The offset function on the Zone class returns positive values west of UTC instead of east like normal. We had something similar with then moment.zone function, which was deprecated in favor of moment.utcOffset.

We should do the same with the Zone object. The offset function should be deprecated, and a utcOffset function should be added, which returns the same value with the sign inverted.

@mattjohnsonpint
Copy link
Contributor Author

This is fairly trivial. A PR would be welcome.

Note, we are just talking about the function. Do not change the offsets array, or the nature of the zone data.

@acrodrig
Copy link

Hi. I am trying to replace the deprecated function, but there is something I do not understand. The following code prints the same offset for both offset and utcOffset. I was expecting the sign to be different (per comments above). Please advise.

const moment = require("moment-timezone");

let zone = null;

zone = moment.tz.zone("America/New_York");
console.log(zone.offset(Date.now()));
console.log(zone.utcOffset(Date.now()));

zone = moment.tz.zone("Asia/Tokyo");
console.log(zone.offset(Date.now()));
console.log(zone.utcOffset(Date.now()));

@AleksMeshkov
Copy link

@maggiepint, I'm sorry. Maybe I don't get something, but what was the point to approve PR if the feature is not ready yet?
image

image

now I get this warnings everywhere but I can't fix them until momentjs will fix invert sign bug :-(

@Alonski
Copy link

Alonski commented Jan 21, 2018

So is this safe to use or not? Should I keep offset or use utcOffset?

@adrian-gierakowski
Copy link

once this gets fixed it will necessitate a major version bump since otherwise utcOffset will break for people who expect current behaviour from it. Could project maintainers confirm that this in fact will be the case so I can replace offset with utcOffset to get rid of the console pollution? thanks!

@Alonski
Copy link

Alonski commented Mar 25, 2018

Hey maintainers... Any updates one this?
@maggiepint

@adrian-gierakowski
Copy link

Hey maintainers... Any updates one this?
@maggiepint, @ellenaua

@JoanYin
Copy link

JoanYin commented Oct 10, 2018

Would like to hear any updates?

@RobLSDev
Copy link

RobLSDev commented Nov 4, 2018

Daylight savings time ending reminded me that we're still waiting on an update for this. @maggiepint @ellenaua

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

No branches or pull requests

7 participants