Moment.js overflows in parsing #235
Comments
|
Sometimes, it seems that having an overflow could be useful, #242. For example, to get the last day of the previous month, you could run: moment().date(0);This is not really an argument against throwing an error, but perhaps the overflow is more useful? @nailor, Is there a reason you need to fail from parsing an incorrect date? |
|
Yes. Failing parsing incorrect date is useful for example if you want to detect if a date entered is incorrect. |
|
In addition to failing on parse, do you think that moment should throw errors elsewhere? For example the I think that JavaScript's error handling is poor compared to other languages, like Python. It would have to look something like this if we used an exception. try {
moment('2011-01-14');
} catch (e) {
if ( e instanceof moment.Exception ) {
alert('failure');
}
// else, probably should re-raise the error;
}I think that there definitely needs to be a parse exception. I had some errors with parsing in a project I was working on and I didn't realize that I had invalid dates because I still received a moment object. For example, the following code does not inform you that it failed: var m = moment('asdf');
// { _d: Invalid Date, _isUTC: false }@timrwood, thoughts? Is it viable to start throwing exceptions when people who have been using the library haven't been expecting exceptions? |
|
Yes, I think the library should clearly throw an exception when it fails to parse things and not default to something obscure, like a moment object with some field set to some funny value. Even though working with exceptions is bit more tedious in JS than in some other languages, it would still make sense to throw up the exceptions to avoid people breaking their software because of rather unexpected results. What comes to changing behavior: bump up version number to new major and announce that the new version has background incompatible changes. |
|
It may be better to add a method to the prototype that checks validity. moment.fn.isValid = function() {
return !!this._d && !isNaN(this._d.getTime());
}This won't throw errors when people aren't expecting them, and people can still check for errors if they dont trust the input data. I think the parser should throw a moment into being invalid, as this matches native js behavior. console.log(new Date('2011-0-100')); //Invalid DateHowever, all |
|
Proposed validation method would not work in this case. For example: moment("2011-01-40").isValid() would return I agree that overflowing is useful, but I'd like to highlight the original problem: Parsing invalid date as a valid in a way, that you really can't be sure (unless, say, you convert it back to expected format and compare). Building a check in parsing that the given date is valid is not too difficult, it can, in the beginning, even be implemented with the naive check of converting date back to the same format and seeing if match the original. What comes to mimicking behavior of the So, if there will be a date parsing in moment.js, why not make it notify about the date validity when parsing? Overflowing would be still possible by passing a array of integers to the moment contsructor. |
|
Sorry, yes, this is what I was referring to with this line.
There should be a check for validity while parsing moment.fn.isValid = function() {
return !!this._isValid && !!this._d && !isNaN(this._d.getTime());
} |
|
|
When doing a parsing on a nonexistent date, moment "overflows", eg.
is parsed as February 10th 2011. The date parser could probably throw an exception or indicate that there's been an error returning null/False or something else, that can't be mixed with a date.
The text was updated successfully, but these errors were encountered: