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

Minified moment-timezone.min.js does not work. #37

Closed
rockerest opened this issue Sep 30, 2013 · 11 comments
Closed

Minified moment-timezone.min.js does not work. #37

rockerest opened this issue Sep 30, 2013 · 11 comments
Labels

Comments

@rockerest
Copy link

Take the following example to force a user's browser to a business' HQ time.

var hqTz        = "America/Chicago",
    rightNow    = moment.tz( hqTz ),
    opens       = moment.tz( hqTz ).hour( 8 ).minute( 30 ).second( 0 ).millisecond( 0 ),
    closes      = moment.tz( hqTz ).hour( 20 ).minute( 0 ).second( 0 ).millisecond( 0 );

console.log( "Local time: " + moment().format() );
console.log( "HQ time: " + rightNow.format() );
console.log( "HQ opens: " + opens.format() );
console.log( "HQ closes: " + closes.format() );

When using the non-minified version, the output is as expected:

Local time: 2013-09-30T07:07:42-10:00
HQ time: 2013-09-30T12:07:42-05:00
HQ opens: 2013-09-30T08:30:00-05:00
HQ closes: 2013-09-30T20:00:00-05:00

However, when using the minified version (the only change made between page loads), the output is incorrect (perhaps as if the timezone data never got loaded):

Local time: 2013-09-30T06:58:27-10:00
HQ time: 2013-09-30T16:58:27+00:00
HQ opens: 2013-09-30T08:30:00+00:00
HQ closes: 2013-09-30T20:00:00+00:00
@nanopils
Copy link

Hi,

I have the same issue and I cannot find why it doesn't work. When the code is not built with r.js, it works like a charm, but when the code gets minified, it is not possible to change timezone (i.e. it stays in the Greenwhich timezone).

Have you found a solution to this issue?

@korondy
Copy link

korondy commented Jan 29, 2014

I just negate the time zone delta – I need GMT+1, so I use moment().zone(-1) and it gets me right time zone.

From: nanopils [mailto:notifications@github.com]
Sent: Wednesday, January 29, 2014 4:45 PM
To: moment/moment-timezone
Subject: Re: [moment-timezone] Minified moment-timezone.min.js does not work. (#37)

Hi,

I have the same issue and I cannot find why it doesn't work. When the code is not built with r.js, it works like a charm, but when the code gets minified, it is not possible to change timezone (i.e. it stays in the Greenwhich timezone).

Have you found a solution to this issue?


Reply to this email directly or view it on GitHub #37 (comment) . https://github.com/notifications/beacon/919599__eyJzY29wZSI6Ik5ld3NpZXM6QmVhY29uIiwiZXhwaXJlcyI6MTcwNjUxNzkyMSwiZGF0YSI6eyJpZCI6MTc2MDM0OTh9fQ==--1fe0221c12b3a7e3f43d350f068c9d5b5cdf76f8.gif

@nanopils
Copy link

@korondy: thanks, but this does not take in consideration the change of summer/winter time which can occur at different times depending on location. I was considering changing the timezone using the zone method, but there are too much factors involved to get it right with this method.

What I would like is to fix the moment-timezone.min bug when minified with require.js. It might be something to add in the require.config file that I'm not aware of?

@nanopils
Copy link

@rockerest: did you find a solution to this issue or did you fix it in another way?

@rockerest
Copy link
Author

I have not discovered a workable solution using the minified version.

To be fair, I spent very little time trying to fix it once I discovered what the issue was. I had already wasted a lot of time up to that point as I had been operating under the assumption that the minified and non-minified versions were functioning identically.

In Moment's defense, I have had similar issues with other libraries' minified versions. It appears that JS minification can be a little finicky.

My solution has simply been to use the "pretty" (unminified) versions of libraries that have stopped working when I minified them. Not ideal, but it's better than trying to debug third-party code when I have better things to do.

@nanopils
Copy link

@rockerest and @korondy: thank you for your feedback!

I use require.js to build moment, moment-timezone and moment-timezone-data, but something got ugly with the uglify2 task and broke the moment-timezone AMD module.

In order to fix this, I have used the already-minified version of moment-timezone (instead of the source code), and it now works a treat!

You can find the minified code I used here: https://raw.github.com/moment/moment-timezone/develop/min/moment-timezone.min.js

@jacwright
Copy link

I believe I've found the problem. I had the same issue as described above. I found that the data script you can download on the moment-timezone homepage is the timezone library WITH the data. It used to be that you had to use both scripts, but now you just use one of those listed on the homepage (the first one is pointless since without data, timezone won't work).

If you include both the timezone script and the data script it will break as indicated in this thread because moment.tz is defined twice. If you only include the data script, everything works.

This isn't very clear in the docs and should be updated. The standalone timezone script should be removed from the homepage as well since it is pointless for anyone to use by itself. The data-builder, when it comes, will probably include the timezone script anyway, and if it doesn't the download for timezone without data can be included on that page instead of the homepage.

@gsvitak
Copy link

gsvitak commented Jul 3, 2014

@jacwright can you please provide a quick sample of your solution. I want to make sure I including the correct dependencies.

@jacwright
Copy link

Just be sure to not load the timezone script twice. For example, when you go to download the script on the timezone home page

screen shot 2014-07-03 at 8 51 29 am

only download ONE of the three links (really, one of the latter two, the first one by itself is worthless). DO NOT download the first one and try to use it with the 2nd or 3rd.

<!DOCTYPE html>
<html>
<head>
<title>Amazing App with Timezone Support</title>
<script type="text/javascript" src="http://momentjs.com/downloads/moment-with-langs.min.js"></script>
<!--
DO NOT USE THIS. It is included with the following script and
by adding it you define timezone twice and end up with subtle
bugs that you aren't able to explain, such as the ones listed in
this thread.
<script type="text/javascript" src="http://momentjs.com/downloads/moment-timezone.js"></script>
-->
<script type="text/javascript" src="http://momentjs.com/downloads/moment-timezone-with-data.js"></script>
</head>
<body>
<h1>Amazing App with Timezone Support</h1>
</body>
</html>

Hope that helps.

@timrwood
Copy link
Member

Yeah, I think we should get rid of the first link and have the zone builder spit out the library with embedded data.

@bitoiu
Copy link

bitoiu commented Jun 7, 2015

I got the same issue and I followed what you said @jacwright, I never even assumed these files could work together so I kept the second version only. If I just try to use the normal moment everyone works fine, and I'll stick to that until I in fact need the timezone feature.

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

No branches or pull requests

7 participants