ngList with uninitialised variable always fails validation with parse: true

[43081j]

since you started all this parse validation business, ngList no longer passes validation unless you initialise the variable it is bound to, to an empty array.

If we have an object:

$scope.model = {};

The HTML:

<input ng-model="model.foo" ng-list=", " name="foo" type="text">

foo will always invalidate the form until you enter something into it, even though it is not required. This shouldn't really be expected functionality as you may have large objects with many properties and only want to add them to the view, rather than some huge object setting them all to null/[].

This is because the ngList parser returns the initial undefined value, which then invalidates the input.

[Narretz]

I cannot reproduce this: http://plnkr.co/edit/eRnly9uIs8mgoUyaXyIX?p=preview with the latest snapshot

[43081j]

hmm strange, ill have a go at it later and post back if i can reproduce it again.

[43081j]

Here we go, it is when a directive validates the input before it has been touched.

http://plnkr.co/edit/Eyam8a7zQcKYIq7b3jni?p=preview

Because on validation, it runs any parsers I assume, which then returns undefined.

[Narretz]

Ah, I see. I think it's reasonable to return an empty array if the viewValue is undefined. There's unfortunately no real rule at the moment how strict the parse errors should be. But since the viewValue is basically empty, we should not set a parse error.

[43081j]

Is there any work around for this? Ideally I don't want to be implementing code for each field to return some default value. Before these parse changes it all worked fine.

I'm pretty much out of ideas now as both my main parser functions break validation simply by returning the passed value.

[Narretz]

In the meantime, you can define your own parser that will treat an undefined viewValue as an empty string: http://plnkr.co/edit/i57n9L76dzJHqG0kSynz?p=preview

Just make sure your parser runs before the ngList parser.

[caitp]

I can't reproduce with 1.2.0, 1.2.22, or 1.0.8 either --- @43081j can you please provide a reproduction for your issue so it can be investigated properly, thanks

[Narretz]

@caitp I already confirmed this via the second plunker.

[43081j]

This one @caitp http://plnkr.co/edit/Eyam8a7zQcKYIq7b3jni?p=preview

This should be tagged as 1.3, sorry I didn't mention that in my initial post (i guess narretz gathered that from the plunkr).

As you can see, parse: false until you change it (add something, then clear it) so it becomes valid. Because the ngList parser returns undefined, if the val is undefined (logically), so i'm not sure how this should be tackled.

[caitp]

heh, so this is a pretty stupid scenario.

I don't think we should change the model (view?) value from undefined to [], that wouldn't make any sense. But the validator pipeline is going to do this stupid thing and treat undefined as invalid, which is bonkers.

There has to be a better way to do this.

[43081j]

Indeed the validators pipeline changes break it. Can you explain why we suddenly made undefined mean invalid?

It isn't ideal initialising every property on fairly large objects. Especially when forms change dynamically but use the same controller and base object.

[caitp]

I think it was for simplicity, but it obviously doesn't make a lot of sense. @matsko we need to do something about this :c

[43081j]

I would understand null meaning it has been skipped (e.g. if nothing was entered yet). But undefined should be a valid value unless a particular validator says otherwise.

Also these changes don't seem to be documented much. So apologies if I misunderstood any of it.

[Narretz]

My discussion senses are tingling.
Why doesn't it make sense to return an empty array in the parser for undefined viewValues? ngList sets an array anyway when a viewValue is entered. This is especially a non-issue if no initial array is defined.

[caitp]

It doesn't make sense because we're changing the value without any input from either a human user or from an actual programatic change in the value --- this is not appropriate.

It just doesn't make sense to do this, we shouldn't be changing peoples values.

For that matter, why are we parse()-ing the model value anyways? In what universe does that make any sense at all? Something is very broken in here

[Narretz]

I don't think it's that broken. The plunker has a directive that validates the model in a watch. That's when the parsing occurs.

[Narretz]

But it's true, an initially undefined value will never make it through $parsers ...

[43081j]

Yes, this isn't exactly specific to ngList. I remember having a similar issue but don't have the code with me, when I used a simple input[type="text"], though it was probably due to me calling $validate beforehand again somewhere.

[caitp]

@Narretz why would you say we're trying to parse the model value when we call validate()? That makes no sense at all, it is broken. Parsing the model value at all, is just nonsense, it makes no sense

[Narretz]

I didn't say we parse the model or modelValue: in the plunker, inside a watch, $validate is called, which calls $$parseAndValidate. There, obviously the viewValue is parsed (which is undefined at the time)

[caitp]

It never makes sense for a viewValue to be undefined when parsing, because the view value should be coming from user input.

it doesn't make sense for us to do anything with undefined view values, certainly not parsing them. This system just does not work.

What I think we want is a simple model in 99% of the app, the model drives the view -- However in the case of user inputs, user inputs drive the model. We should not be formatting model values which have not changed, we should not be parsing view values that have not changed. There is no reason for either of these to occur.

We listen for model value changes with $watch, and we have APIs for telling the system when the view value has changed. Therefore, we have a way to know when to format, and we have a way to know when to parse. There is no need to ever "automatically parse() because we're validating", that just makes zero sense at all.

[caitp]

If I carry on within this conversation it's going to turn into one of those nasty discussions where people think I'm yelling at them, and I don't want that to happen. Everyone has worked hard on this =)

But, I still think we are not doing the right thing here. We shouldn't be re-parsing when the viewValue hasn't changed (we shouldn't be re-parsing at all, because this should happen in the watch listener).

[Narretz]

All good points. You should bring this up to the core developers. I'm actually surprised you weren't assigned the input / validation refactoring. At the moment, it looks like @tbosch is assigned, so you could also discuss it with him.

[Narretz]

@tbosch are you still quasi assigned to form validation? If not, I can take a shot at submitting a PR for this.

[43081j]

Just a small update.

This issue also exists even when the model is null. If I use a $resource and it comes back with a property set to null, there will be a parse: true error if any parsers exist on it.

This is due to $$parseAndValidate only checking the view value, which is of course undefined if the model value came back null.

So even where I have models with their properties all defined, a null causes invalid form if there's a parser on that particular field (due to the fact that $$parseAndValidate ignores undefined EXCEPT when a parser exists and returns it, even if the parser returns what it is given).

Making undefined mean invalid has broken so many things and caused so many problems, I begin to wonder why it was even changed.

[caitp]

This might have been fixed by 92f05e5 --- lets find out :3

[caitp]

http://plnkr.co/edit/7S1B0sZkSeTXEl8MbDJE?p=preview looks like it is working! I should have tracked down this issue too