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

format differs between linux and windows #272

Closed
katowulf opened this issue Apr 10, 2012 · 10 comments
Closed

format differs between linux and windows #272

katowulf opened this issue Apr 10, 2012 · 10 comments

Comments

@katowulf
Copy link

My testmoment.js script:

var moment = require('moment');

moment.utc('04/22/2012').format(moment.isoFormat);

When I run this on Windows I get 07:00

"C:\Program Files\nodejs\node.exe" adhoctests\testmoment.js
2012-04-22T07:00:00+00:00

When I run this on Linux I get 00:00

[user ~]$ node adhoctests/testmoment.js
2012-04-22T00:00:00+00:00

I'd be happy to see if I can fix this up if you point me in the right direction.

@timrwood
Copy link
Member

The string parser falls through to the native date parser (essentially Date.parse()) if the string does not match ISO8601.

Different browsers (and I guess different OS now) support different strings in Date.parse. Linux is probably parsing into UTC, and windows is parsing into local time.

If you know the format of your input, you can add a parsing token using moment.utc('04/22/2012', 'MM/DD/YYYY')

@katowulf
Copy link
Author

My objective is to create a toDate() that will handle common formats, to parse/validate user input.

It does seem to be local time on Windows, considering I'm at GMT - 7.

Since I'm not calling this from a browser, but from the node executable, does this mean it's an issue with node or V8's Date.parse?

@timrwood
Copy link
Member

Yeah, you may want to just try something like this in windows/unix to see if that is the problem.

console.log((new Date('04/22/2012')).toString());

@katowulf
Copy link
Author

It's odd, but it seems to be based on the computer's time zone. I noted that the Linux box was set to UTC, so I set the Windows box to UTC to run a test. It literally returns a number of hours equal to my offset from UTC when no time is specified.

var moment = require('moment');

console.log('new Date()', new Date('04/22/2012'));

var utc = moment.utc('04/22/2012');
console.log('hours', utc.hours());
console.log('moment.format', utc.format(moment.isoFormat));

Computer set to UTC time:

new Date() Sun, 22 Apr 2012 00:00:00 GMT
hours 0
moment.format 2012-04-22T00:00:00+00:00

Computer set to GMT -7:

new Date() Sun, 22 Apr 2012 07:00:00 GMT
hours 7
moment.format 2012-04-22T07:00:00+00:00

Hrmmmphrgle...

I'm sure you aren't looking for extra things to do, but it seems to me submitting a date-only value should parse consistently to 00:00 for hours, regardless of the server's time zone.

It's a bit annoying to go put a library on top of moment that will read an incoming date, decide if it matches a valid format, and then parse it.

@katowulf
Copy link
Author

Looks like this behavior is changing again in node.js 0.7.x: http://stackoverflow.com/questions/9959965/node-js-returns-gmt-time-and-not-local-time-for-new-date-is-that-a-bug

@timrwood
Copy link
Member

Yeah, if it's falling through to the native Date bugs/inconsistencies, than there's not much we can do.

It would be nice to create a 'magic' string parser that takes into account language, big/small endian, etc, but it would bloat the library considerably, and I don't think it's a common enough use case to warrant putting it in core.

It would be nice as a plugin though...

@katowulf
Copy link
Author

A plugin for parsing different date formats does sound like a good solution.

This behavior is more far-reaching than just unspecified formats, though. For instance, moment(String) shows that it accepts these formats:

"YYYY-MM-DD"
"YYYY-MM-DDTHH"
"YYYY-MM-DDTHH:mm"
"YYYY-MM-DDTHH:mm:ss"
"YYYY-MM-DDTHH:mm:ss z"

However, I have the same issue if I use anything except for YYYY-MM-DDTHH:mm:ss z (even specifying a time of 00:00!); I can't run the same test units on Linux and Windows (they fail on one or the other).

      it('parsing `2012-04-222012-04-22T00:00:00`',function() {
         expect(util.toDate('2012-04-22T00:00:00').format(moment.isoFormat)).to.be('2012-04-22T00:00:00+00:00');
      });

Passes if my server uses UTC timezone, fails if not:

  2) util #toDate() parsing `2012-04-222012-04-22T00:00:00`:
     Error: expected '2012-04-22T07:00:00+00:00' to equal '2012-04-22T00:00:00+00:00'

I understand this is probably a lot to think about, and it doesn't look like a quick update to change, but I would encourage you to consider normalizing this behavior.

I don't see this any different than normalizing browser differences. If our test units can't run in both environments, well there's going to be frowny faces all around.

And since I'm whining about it, and I would like to use moment with our project, let me know how I could assist.

@katowulf
Copy link
Author

And I guess I would append that regardless of whether it's using the underlying Date.parse(), I do feel that 2012-04-222012-04-22T00:00:00 should always set the hour to 00 and never to 07, particularly since the spec says this is an accepted ISO format.

@timrwood
Copy link
Member

I'm not sure what util.toDate is doing, but that may be the problem.

Momentjs has two modes, local time, and utc time. This affects the display options, like moment.fn.hours and moment.fn.format.

By default, ISO8601 strings and strings parsed with a known format are put into local time. Basically, anything passed to moment() will be in local mode.

Anything parsed with moment.utc() will be put into utc mode. jsfiddle of the below

moment("10", "H").hours(); // in local mode, should be 10 PST (or your timezone)
moment.utc("10", "H").hours(); // in utc mode, should be 10 GMT
moment("10", "H").utc().hours(); // parsed in local, switched to utc, should be 17 GMT (10 + offset)
moment.utc("10", "H").local().hours(); // parsed in utc, switched to local, should be 3 PST (10 - offset)

It does look like there is a bug in the single string parser for moment.utc()

moment.utc('2011-10-10T10').format('H Z'); // should be 10, is 17

I think the bug is on this line. It's sending the string to moment() to parse, but because the timezone is not added, it is being parsed as local time. It should be something like the following.

return (format && input) ? moment(input + ' 0', format + ' Z').utc() : moment(input + '+0000').utc();

I'll open a separate issue for the moment.utc(string) issue for easier tracking.

@katowulf
Copy link
Author

Yes, I think we've moved to the heart of it now. If I were a bit more articulate, maybe we could have started there : )

And I think you're right; I think '+00:00' should fix it up nicely.

FYI, in the example above, util.toDate(date) is just calling moment; forgot to extract that for examples.

util.toDate = function(o, def) {
   var d = moment.utc(o);
   if(!d || d._d == 'Invalid Date') {
      return def;
   }
   return d;
}

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

No branches or pull requests

2 participants