input[type=date] borks model on binding if ngRequired is falsey

[EverPresent]

So this wasn't a problem in the previous version of angular that I was running (1.2.*?) but as soon as I upgraded to 1.3.1 all of my required date input types would set my model value to null upon binding (which would fire my $watches prematurely and cause havok).

It seems there have been a lot of input controller related issues arise in 1.3 but looking through the open issues I don't think this one has any related.

Check out this plunk to verify the issue:
http://plnkr.co/edit/dgsiLXD4OTDt19FF6pmi?p=info

I tracked down the problem to the default formatter that is attached to the input in input.js:createDateInputType. It is returning an empty string if the value evaluates to isEmpty. Looking at other formatters the rule of thumb seems to be to return the original value that evaluated to falsey (not change it to empty string). I made this one line change and if you click the "see fixed version" at the bottom of the plunk you will see the exact same page using my fixed version of the angular.js file.

[Narretz]

I think the reason for returning empty string is to have equality between $viewValue and actual DOM value. IIRC browsers that support date won't set invalid date strings as values - though that could be the case for throwing actually.

[frfancha]

Set $viewValue to '' to match DOM, why not, but then why changing the model?

[Narretz]

"changing the model" is the bug

----- Ursprüngliche Nachricht -----
Von: "frfancha" notifications@github.com
Gesendet: ‎11.‎11.‎2014 19:46
An: "angular/angular.js" angular.js@noreply.github.com
Cc: "Narretz" mjstaffa@googlemail.com
Betreff: Re: [angular.js] input[type=date] borks model on binding ifngRequired is falsey (#9996)

Set $viewValue to '' to match DOM, why not, but then why changing the model?
—
Reply to this email directly or view it on GitHub.=

[frfancha]

Ok.

[EverPresent]

Mmmm... Ok, but the formatter in the stringBasedInputType function returns the original value if the value evaluates to isEmpty (which is used by input[type] text, email, and url).

function stringBasedInputType(ctrl) {
  ctrl.$formatters.push(function(value) {
    return ctrl.$isEmpty(value) ? value : value.toString();
  });
}

As does the one in the numberInputType function (used by input[type] number).

  ctrl.$formatters.push(function(value) {
    if (!ctrl.$isEmpty(value)) {
       ... omitted for brevity ...
    }
    return value;
  });

The only other built-in formatters that I found in the angular code were for checkbox and ngList which are both kind of special cases anyway.

Is matching the DOM value something that is needed? If so, it would seem to me that these other formatters for those input types mentioned should be modified to return an empty string as well. If not, then returning the original value would seem to be an adequate fix. Thoughts?

[Narretz]

@EverPresent this sounds like a good idea, actually. What makes me wonder though: when the model gets formatted to undefined instead of '', it should still get converted to null, because it is still empty. I'll have to test this again.

[EverPresent]

I'm playing around with this and thought I would post a little more info for your consumption.

When the formatter receives undefined and formats it into an empty string. The parser immediately turns around and formats that empty string into null. The rest of the angular framework sees that the value of that field is set to null, and modifies the model accordingly.

The crux of this bug, I think, is occurring when the parser converts the empty string into a null value. The rest of the angular framework is just taking that value, and doing what it is supposed to do. If the parser always converts an empty string into null, then the angular framework will always modify the underlying model (even prior to user action as we note in the above plunk) because null is a legitimate value for a bound property. If the parser always converts an empty string into undefined then the angular framework will delete the property from the model object (not necessarily what we want to happen either).

Both null and undefined evaluate to true in isEmpty. I think the formatter must return the original value because there is a big difference between these values. If the formatter returns undefined then the parser is never called and no model-changing actions occur.