ng-messages no longer interprets its attribute value properly

[davidwalschots]

In order to minimize the amount of html needed for validation, at our office, we created a directive on top of ng-messages. Sadly, moving from 1.4.0-beta.4/angular-messages.js to 1.4.0-beta.5/angular-messages.js (and later version). Our code no longer works.

The error is as follows:

Syntax Error: Token '{' invalid key at column 2 of the expression [{{::messages.fullFieldName}}.$error] starting at [{::messages.fullFieldName}}.$error].

The template for this directive is as follows:

<div class="validation-message" ng-messages="{{::messages.fullFieldName}}.$error" ng-show="{{::messages.fullFieldName}}.$invalid && !{{::messages.fullFieldName}}.$pending && ({{::messages.formName}}.$submitted || {{::messages.fullFieldName}}.$touched)">
  <div ng-transclude></div>
</div>

The directive is defined as follows:

function indMessages() {
    var controller = function ($element) {
      var vm = this;
      var form = $element.inheritedData('$formController');

      // Unfortunately required controllers are not injectable into our own controller, only into our link function.
      // Here's a solution, feels a bit hacky but nicer than creating a link function just for this; based on: http://stackoverflow.com/a/21252383/198797; also used in core angular code: https://github.com/angular/angular.js/blob/adcc5a00bf582d2b291c18e99093bb0854f7217c/src/ng/directive/input.js#L1614
      vm.formName = form.$name;
      vm.fullFieldName = vm.formName + '.' + vm.fieldName;
    }

    controller.$inject = ['$element'];

    return {
      restrict: 'E',
      require: '^^form',
      templateUrl: '/views/templates/ind-messages.html',
      bindToController: {
        fieldName: "@for"
      },
      controllerAs: 'messages',
      transclude: true,
      replace: true,
      scope: true,

      controller: controller
    };
  }

Is the above due to a programming error on our part? Or was there a change with unexpected side-effects inside the angular-messages.js code?

[Narretz]

Can you please provide a runnable example inside a plnkr or jsfiddle? Thanks!

[davidwalschots]

Of course! It can be found here: http://plnkr.co/edit/p1bHusDA7ZyXhuW9Ymz9

[gkalpak]

You could make it work by changing:

ng-messages="{{::messages.fullFieldName}}.$error"

to:

ng-messages="::this.$eval(messages.fullFieldName)['$error']"

(updated Plnkr).

But I am not sure this is a nice solution.

[singhsays]

I'm seeing this issue are well. No custom directive or anything, just plain vanilla ng-messages. It seems to me, that the ng-messages directive no longer interpolates the ng-messages and for attribute values.

Demo:
http://jsfiddle.net/singhsays/m06jeth1/ (works fine with 1.3.15, upto 1.4.0-beta.4)
http://jsfiddle.net/singhsays/oq473qyq/ (same code, stopped working at 1.4.0-beta.5)

[tkrotoff]

+1, I have the same problem

[sravenhorst]

This issue is basicly this:

in Angular 1.4.0.beta.4 the ngMessages/for attribute were read in the (post)link function. Here the "dynamic" value was already parsed before read in the (post)link.

In later Angular 1.4.X versions, the value in the ngMessages/for attribute is already read in the controller function, where the attribute isn't parsed yet.

@Narretz, as you set the status of this issue:
The solution would be to link the ngMessages/for attributes through bindToController so it's parsed before read.
I hope enough info is applied to pick this up.

A workaround we've introduced for now in our app, is something like you find in this fiddle: http://jsfiddle.net/m06jeth1/20/

So basicly: we added a custom directive to set the "for" attribute dynamicly.
But as said: this is a workaround and i hope the real issue will be solved within angular-messages.js.

Groet,
Sander

[tkrotoff]

@sravenhorst Your jsfiddle (http://jsfiddle.net/m06jeth1/20/) is missing something: it never displays "This is a required field"

[patrickjuchli]

I have the same issue, until then:

ng-messages="this[messages.fullFieldName].$error"

works, too.

[tkrotoff]

Here another demo that shows the regression: http://plnkr.co/edit/BOYfaF?p=preview

Personally, I dynamically patch AngularJS using the postinstall step of Node.js:

File patches/angular-messages-#12001.diff (created with diff -u):

--- ../node_modules/angular-messages/angular-messages.js
+++ ../node_modules/angular-messages/angular-messages.js
@@ -325,8 +325,13 @@
      var INACTIVE_CLASS = 'ng-inactive';

      return {
-       require: 'ngMessages',
        restrict: 'AE',
+       bindToController: {
+         ngMessages: '@',
+         forAttr: '@for'
+       },
+       scope: true,
+       controllerAs: 'ngMessagesController',
        controller: ['$element', '$scope', '$attrs', function($element, $scope, $attrs) {
          var ctrl = this;
          var latestKey = 0;
@@ -392,7 +397,7 @@
               : $animate.setClass($element, INACTIVE_CLASS, ACTIVE_CLASS);
          };

-         $scope.$watchCollection($attrs.ngMessages || $attrs['for'], ctrl.render);
+         $scope.$watchCollection(ctrl.ngMessages || ctrl.forAttr, ctrl.render);

          this.reRender = function() {
            if (!renderLater) {

File package.json:

  "dependencies": {
    "angular": "~1.4",
    "angular-messages": "~1.4"
  },
  "scripts": {
    "patch": "for i in patches/*.diff; do patch -p1 < $i; done",
    "postinstall": "npm run patch",
  }

This way I can easily change my version of AngularJS and include other patches if needed instead of waiting for a new release (this regression is 6 months old...).

[Narretz]

I know there's a PR open for this, which needs some improvement. Would you like to give it a shot @tkrotoff?

[tkrotoff]

@Narretz I've looked at the PR but I don't have the knowledge to "fix" it

[Narretz]

I had a look at this, and I'm not really sure anymore that this is (a) a regression and (b) something that really makes sense - I can definitely see the use, but the semantics are a bit more complicated.

For (a), it's not been documented that you can interpolate the ngMessages collection. And (b), interpolating inside an expression before it is evaluated is not something Angular really advocates. For example, if interpolating the expression were possible, wouldn't you expect that ngMessages picks up this change? For example, you have ng-messages="{{collectionName}}" and you decide later on that you want to use a different set of message keys ( for whatever reason).
In the patch, and before, this change wouldn't be picked up, because the watchCollection would settle down to watch the expression with the result of the first interpolation.

I see that in the examples, the collection name would probably never change, but it's also questionable if we should support a case that's not really angular like, i.e. having interpolation inside another expression. I'd like to hear what others think about this. @matsko wdyt?

[Narretz]

To solve this issue, I would simply move the logic of assembling the path to the collection into the directive controller: http://plnkr.co/edit/u6XUZbHr2vUcdQGfe0IM?p=preview

Or you could also expose the form to the directive and use standard brackets notation to access the field: messages.form[messages.fieldName].$error And actually, you could simply expose the field ... this is probably the easiest way: messages.formField.$error

There also this workaround if you insist on the way it's done in the example: this.$eval(messages.fullFieldName).$error, see #11616 (comment)

Interpolation also has a performance penalty, even if it is one-time binding; It's also more readable as a pure expression,

[JohnColvin]

@Narretz thanks for example. It helped me convert a directive that had this issue. 🎉

[petebacondarwin]

I agree with @Narretz here. I think that this is not something that we should support. We definitely do not encourage interpolation in attributes of directives that expect an expression.

[Narretz]

Okay, I'll add a note then to the changelog & the migration docs, and close this afterwards.

[petebacondarwin]

Thanks!

[brauliodiez]

About both solutions proposed, I have tried adding two validations (required and minlength), and it always displays the required text message (you never get the minLength one).

      <input ind-input type="text" 
      name="testField" ng-model="testField" required
      ng-minlength="10"/>

      <ind-messages for="testField">
        <div ng-message="minlength">min length.</div>
        <div ng-message="required">Fill in the text field.</div>
      </ind-messages>

Plunkr based on @Narretz proposal

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

Plunkr based @gkalpak proposal

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

I'm doing something wrong or missing something?

[gkalpak]

In contrast to minlength, ngMinlength expects an Angular expression (not a literal value).
In order for your example to work, you should replace ng-minlength="10" with either:

1. minlength="10" (updated plnkr 1) or
2. ng-minlength="'10'" (updated plnkr 2)

[brauliodiez]

Ouch, sorry about that.

[mohitadwani]

In which version will this be released?

[petebacondarwin]

@mohitadwani - will what be released?

[mohitadwani]

@petebacondarwin the code resolving this issue. In which version will this be released and when ?

[petebacondarwin]

There is no code resolving this issue.
The only related commit is aff74ec, which contains only documentation of the issue and why this issue will not be fixed.
See #11616 (comment)

[mohitadwani]

@petebacondarwin Sorry, I hadn't checked it properly. Thanks

[tkrotoff]

@petebacondarwin

this is not something that we should support

@Narretz

I would simply move the logic of assembling the path to the collection into the directive controller

You are saying that ng-messages should not support things like "form{{$index}}..." anymore while ng-show does (and probably other directives) => seems unlogical to me.

Interesting part (simplified) from the plunker given in my comment above:

<form name="form{{$index}}"
      ng-repeat="participant in participants">

  <input name="name" required
         ng-model="participant.name">

  <span ng-messages="form{{$index}}.name.$error" // <= not working anymore since 1.4
        ng-show="form{{$index}}.name.$invalid">
    <span ng-message="required">Cannot be empty</span>
  </span>
</form>

[petebacondarwin]

I don't think we should support it in ng-show either but it happens to work. In my view this is too many levels of indirection.

In any case you can achieve what you want, as discussed earlier in this issue with:

<span
    ng-messages="this['formParticipant' + $index].name.$error"
    ng-show="this['formParticipant' + $index].name.$invalid"
    class="help-block">

See http://plnkr.co/edit/ba09QK?p=preview

[tkrotoff]

Cool, thx. I was afraid to have to implement a new directive like given in a comment above.

[rhclayto]

How then do you make this work without {{}} syntax when you are dynamically generating fields & field names with ng-repeat, not when dynamically generating form names, as in the examples I have seen up till now?

For instance, this plunker works with a manually defined field name attribute in ng-message: http://plnkr.co/edit/Wswcbu?p=preview

However, doing something as in this plunker does not work: http://plnkr.co/edit/L3iyZE?p=preview

This would be easy with the ability to use the {{}} interpolation syntax, but without it I am unsure of how to accomplish this. All examples I have seen involve doing something like ng-messages="this['formParticipant' + $index].name.$error", but don't address the case of dynamic field names.

Edit: Nevermind, I figured it out: <span ng-messages="formParticipant['name' + $index].$error" ng-show="formParticipant['name' + $index].$invalid" class="help-block"> works.