Setter functions should handle garbage input better

[mattjohnsonpint]

As discussed in #3436, the setter functions like year don't handle garbage input well.

var m = moment().year('foo');
m.isValid() // true
m.format()  // "-0NaN--NaN--NaNT-NaN:-NaN:-NaN+00:00"

Same goes for many of the other setters, such as date and hour. They should all be checked.

Since many other functions, including add will fail silently on garbage input, these probably should also. The alternative would be to throw an error, which would be inconsistent with the rest of the library.

[maggiepint]

Silent fail. Probably be quite doable as up-for-grabs.

[mbad0la]

Hi @mj1856 @maggiepint !
Would this issue be an easy one for someone who wants to understand the architecture of this module? I want to start contributing regularly to moment and I'd love to take up issues under guidance until I've explored the module enough.

[maggiepint]

@mbad0la probably. Basically, we need to validate that the input to a set function is of type number before we actually run the set on the date that is contained internally.

[mbad0la]

@maggiepint let me try this then? I'll let you know of my progress.

[wi-ski]

Hi Hi - I was looking into this and was wondering what the status was. It looks like CI failed for last attempt by @mbad0la. Would anyone mind if I took a stab at it? I dont want to step on anyone's toes ^^

[mbad0la]

Hi @willdembinski

I'm almost done with the PR. Did you follow the conversation on the PR thread?
Anyway, I'll notify you if I'm still not able to resolve this bug 🙂

[wi-ski]

I did read the PR thread. Hence the question 😝 🐛

[maggiepint]

@willdembinski we have had a few bugs come in in the last few weeks that would be good starters. Maybe look at #3430 or #3463. I"ll up for grabs both of those.

[wi-ski]

Maggie!

Youre so sweet :) Thanks for doing that. Ill take a look when Im out of
the office <3

[wi-ski]

I had a quick question. "Overflowing" in the context of moment.js:

Is that when one adds a unit of time to some component of time (1 hour,1
minute, or 1 second) and causes the next...order of magnitude(?) to
increment?
Eg. I add one minute to Wed Oct 12 15:59:00 PDT 2016 resulting next
state would be: Wed Oct 12 16:00:00 PDT 2016.

OR is it when moment("2011-01-40") => Feb 10 2011? Do you have a word for
the difference?

[mattjohnsonpint]

@willdembinski - typically when we talk about overflow/underflow, we mean the latter. Your first example is just a one minute addition.

[mattjohnsonpint]

Though also note that it would be Feb 9th, and we don't actually allow for overflow in the constructor like that in moment. It would just be an invalid date.

[wi-ski]

@maggiepint @mj1856 @mbad0la - I wanted to swing back around on this and (with utmost respect to whomever may have already put forth effort) ask if it is ok to take a poke at it (or if its still a valid opportunity). :)

[maggiepint]

@willdembinski go for it. You can start from #3486 and revise as needed, or rewrite. But do look at the feedback on there.

[wi-ski]

@maggiepint - When you have the time, please check out #3704. I feel like it could be better? JS and its coercion shenanigans never fail to amaze me.

[maggiepint]

Closed in favor of #3704