ngMessages blows up when there are duplicate child ngMessage keys #9338

Closed
niallsmart opened this Issue Sep 30, 2014 · 5 comments

Projects

None yet

3 participants

@niallsmart
Contributor

The ngMessages directive gets into an invalid state and throws an exception when there are duplicate ngMessage child keys specified. For example:

<div ng-messages="form.id.$error">
  <div ng-message="required">
    User ID is required.
  </div>
  <div ng-message="required">
    User ID is required.
  </div>
</div>

The issue seems to be a bug in the re-ordering logic in registerMessage which is attempting to preserve the ordering from an included template, but in this scenario inserts null into messages.

Test case here: http://plnkr.co/edit/7u4oFTGbeLqa1tmfMNlJ (look at the console log).

@btford
Contributor
btford commented Sep 30, 2014

I don't think we want to support this behavior. What's your use-case?

@btford btford added this to the Purgatory milestone Sep 30, 2014
@niallsmart
Contributor

@btford it was actually a typo, but I expected to see it handled more gracefully :)

@btford
Contributor
btford commented Sep 30, 2014

@IgorMinar do you think it makes sense to improve the error message in this case?

@matsko
Member
matsko commented Nov 3, 2014

It should just silently ignore the 2nd occurrence.

@matsko matsko added a commit to matsko/angular.js that referenced this issue Jan 7, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This fix ensures that both
the ngMessage and ngMessages directives automatically update when
any dynamic message data changes.

BREAKING CHANGE:

The ngMessage directive now uses expressions. Therefore, all
pre-existing ngMessage attribute values (as well as the values
assigned via the `when` attribute) will not function as they did
before because ngMessage will evaluate the attribute value as an
expression. A quick fix for this is to wrap single quotes around each of
the attribute values:

```html
<!-- AngularJS 1.3.x -->
<div ng-message="required">Your message is required</div>

<!-- AngularJS 1.4.x -->
<div ng-message="'required'">Your message is required</div>
```

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="'required'">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
3e3634d
@matsko matsko added a commit to matsko/angular.js that referenced this issue Jan 8, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This fix ensures that both
the ngMessage and ngMessages directives automatically update when
any dynamic message data changes.

BREAKING CHANGE:

The ngMessage directive now uses expressions. Therefore, all
pre-existing ngMessage attribute values (as well as the values
assigned via the `when` attribute) will not function as they did
before because ngMessage will evaluate the attribute value as an
expression. A quick fix for this is to wrap single quotes around each of
the attribute values:

```html
<!-- AngularJS 1.3.x -->
<div ng-message="required">Your message is required</div>

<!-- AngularJS 1.4.x -->
<div ng-message="'required'">Your message is required</div>
```

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="'required'">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
b91eb65
@matsko matsko added a commit to matsko/angular.js that referenced this issue Jan 8, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This fix ensures that both
the ngMessage and ngMessages directives automatically update when
any dynamic message data changes.

BREAKING CHANGE:

The ngMessage directive now uses expressions. Therefore, all
pre-existing ngMessage attribute values (as well as the values
assigned via the `when` attribute) will not function as they did
before because ngMessage will evaluate the attribute value as an
expression. A quick fix for this is to wrap single quotes around each of
the attribute values:

```html
<!-- AngularJS 1.3.x -->
<div ng-message="required">Your message is required</div>

<!-- AngularJS 1.4.x -->
<div ng-message="'required'">Your message is required</div>
```

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="'required'">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
8ece832
@matsko matsko added a commit to matsko/angular.js that referenced this issue Feb 11, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
912b03f
@matsko
Member
matsko commented Feb 11, 2015

#10676 will fix this for 1.3 and 1.4

@matsko matsko added a commit to matsko/angular.js that referenced this issue Feb 12, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
5d55976
@matsko matsko added a commit to matsko/angular.js that referenced this issue Feb 12, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
c118182
@matsko matsko added a commit that closed this issue Feb 12, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
c9a4421
@matsko matsko closed this in c9a4421 Feb 12, 2015
@matsko matsko added a commit to matsko/angular.js that referenced this issue Feb 12, 2015
@matsko matsko feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338

Conflicts:
	src/ngMessages/messages.js
d97733f
@netman92 netman92 added a commit to netman92/angular.js that referenced this issue Aug 8, 2015
@matsko @netman92 matsko + netman92 feat(ngMessages): provide support for dynamic message resolution
Prior to this fix it was impossible to apply a binding to a the
ngMessage directive to represent the name of the error. It was also
not possible to use ngRepeat or any other structural directive to
dynamically update the list of messages. This feature patch ensures
that both ngMessages can render expressions and automatically update
when any dynamic message data changes.

BREAKING CHANGE:

The `ngMessagesInclude` attribute is now its own directive and that must
be placed as a **child** element within the element with the ngMessages
directive. (Keep in mind that the former behaviour of the
ngMessageInclude attribute was that all **included** ngMessage template
code was placed at the **bottom** of the element containing the
ngMessages directive; therefore to make this behave in the same way,
place the element containing the ngMessagesInclude directive at the
end of the container containing the ngMessages directive).

```html
<!-- AngularJS 1.3.x -->
<div ng-messages="model.$error" ng-messages-include="remote.html">
  <div ng-message="required">Your message is required</div>
</div>

<!-- AngularJS 1.4.x -->
<div ng-messages="model.$error">
  <div ng-message="required">Your message is required</div>
  <div ng-messages-include="remote.html"></div>
</div>
```

Closes #10036
Closes #9338
6ac5959
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment