ngAria ngModel directive breaks when used with terminal priority 1<p<201 directive

[Chryssie]

Note: for support questions, please use one of these channels: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#question. This repository's issues are reserved for feature requests and bug reports.

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

When using the ngAria module with a terminal, priority 1<p<201 directive on the same element as an ngModel directive an error occurs breaking the compile step.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://plnkr.co or similar (template: http://plnkr.co/edit/tpl:yBpEi4).

1. Include ngAria
2. Define directive with terminal: true and a priority 1<p<201
3. Apply defined directive and ngModel to the same element
4. Run application, error occurs

See http://plnkr.co/edit/amf20s9OovC3vYzjOKhU

What is the expected behavior?

No error should occur.

What is the motivation / use case for changing the behavior?

Which versions of Angular, and which browser / OS are affected by this issue? Did this work in previous versions of Angular? Please also test with the latest stable and snapshot (https://code.angularjs.org/snapshot/) versions.

1.4.8 does not have the issue. 1.5.5 and the snapshot have it.

Other information (e.g. stacktraces, related issues, suggestions how to fix)

Issue appears to have introduced in the commit a460ce4 with a change to src/ngAria/aria.js

[gkalpak]

It didn't exactly work previously either (it would still fail in certain cases, e.g. for checkbox-shaped inputs). It might be an issue with the $compiler itself.

Nice report and demo btw 💯
Really concise and yet thorough 👍

[Narretz]

This is technically expected behavior, as priority 10 and terminal means that ngAria's ngModel (priority 200) is added, but the core ngModel is not (priority 1).

Since ngAria's ngModel is undocumented, I suggest we should safeguard against this happening by making the requirement on the ngModelController in ngAria's ngModel optional and bail out completely when it is not found.

[gkalpak]

@Narretz, it's not that it isn't there. It is just compiled later, which means it is not available in pre-linking phase, but it is available during post-linking. Here is the sequence of operations:

Directive	Phase	...
aria-ngModel	compile	(because it has a higher priority than foo)
foo	compile	(terminal: no more directives to be compiled)
aria-ngModel	pre-link	(at this point the code-ngModel controller has not been instantiated)
foo	pre-link
foo	post-link	(compiles core-ngModel)
core-ngModel	compile
core-ngModel	controller	(hereafter the NgModelController will be available)
core-ngModel	pre-link
core-ngModel	post-link
aria-ngModel	post-link	(the NgModelController is available here)

There are a couple of different ways to solve this, but after digging into it a bit, I think that we should not actually be providing a custom $isEmpty method for checkboxes. We are re-using the method from input[checkbox], but contrary to input[checkbox] we are not defining custom parsers/formatters, which makes it pointless. For example, if the user doesn't specify a value and leaves it as undefined, the custom checkbox would be considered "checked", which is certainly unexpected.

I suggest we should remove the custom $isEmpty method in 1.6 (with a breaking change notice) and instruct people to provide their own (along with custom parsers/formatters) if they need to.
@Narretz, since you were the one to introduce this (in 44337f6), WDYT ? (Maybe I have missed something.)

[Narretz]

it's not that it isn't there. It is just compiled later, which means it is not available in pre-linking phase, but it is available during post-linking.

I don't think this is right. When you have a directive with priority: 10 and terminal, the core ngModel directive doesn't get added, which means the ngModelController is not available, ever. With terminal, there is no "compiled later". Or do you mean in foo's postLink fn (which is specific to OP's example)?

[Narretz]

@Chryssie I wonder what's your use case. Why does your directive have terminal and priority: 10 Do you want to execute some directives and not others? And why do you re-compile in the postLink fn?

[gkalpak]

With terminal, there is no "compiled later". Or do you mean in foo's postLink fn (which is specific to OP's example)?

Yes, I meant with manually continuing compilation later.
If a directive is terminal and doesn't compile all directives, we would end up with aria-ngModel but not core-ngModel, which would not work correctly, since aria-ngModel depends on core-ngModel. So throwing an error in that case is desired.