New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow duplicate objects in ngRepeat. #2505

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
@spamdaemon

spamdaemon commented Apr 25, 2013

Currently, ngRepeat doesn't allow modification of a list that contains
duplicates. For example, while ng-repeat='i in [1,1,1]' is allowed, it
is not possible to add a 1 to the list after it has been iterated at
least once. This patch addresses this issue.
Just by observing a change from [1,1,1] to [1,1,1,1] it is not possible
to know the position where the additional 1 was inserted (vice versa for
removal of 1). For the purposes of this patch, we assume that the item
was inserted after all other duplicates (or is the last duplicate to be
removed).
This patch does have a visible effect when using animations. Consider
the following list [a,b,a] and inserting a in position 0 to yield
[a,a,b,a]. Since we're assuming that a was added, we need to apply a
series of moves to yield the desired list:

  • a in position 0 does not change
  • a moves from position 2 to position 1
  • b moves from position 1 to position 2
  • a new a is added in position 3
    It is probably possible to minimize the number of moves necessary, but that
    would come at some cost.
    This patch assumes that no two objects that are not occupying the same
    memory location will have the same $$hashKey (which is the same
    assumption of the previous implementation but it was not stated!)
R. Merkert
Allow duplicate objects in ngRepeat.
Currently, ngRepeat doesn't allow modification of a list that contains
duplicates. For example, while ng-repeat='i in [1,1,1]' is allowed, it
is not possible to add a 1 to the list after it has been iterated at
least once. This patch addresses this issue.
Just by observing a change from [1,1,1] to [1,1,1,1] it is not possible
to know the position where the additional 1 was inserted (vice versa for
removal of 1). For the purposes of this patch, we assume that the item
was inserted after all other duplicates (or is the last duplicate to be
removed).
This patch does have a visible effect when using animations. Consider
the following list [a,b,a] and inserting a in position 0 to yield
[a,a,b,a]. Since we're assuming that a was added, we need to apply a
series of moves to yield the desired list:
* a in position 0 does not change
* a moves from position 2 to position 1
* b moves from position 1 to position 2
* a new a is added in position 3
It is probably possible to minimize the number of moves necessary, but that
would come at some cost.
This patch assumes that no two objects that are not occupying the same
memory location will have the same $$hashKey (which is the same
assumption of the previous implementation but it was not stated!)
@mhevery

This comment has been minimized.

Show comment
Hide comment
@mhevery

mhevery Apr 25, 2013

Member

This works as intended. If there is a need for duplicate entries then we can no longer keep association between the DOM and the entry and so the only way to associate is by position.

ng-repeat='i in [1,1,1] track by $index' will do what you want.

Closing the issue.

Member

mhevery commented Apr 25, 2013

This works as intended. If there is a need for duplicate entries then we can no longer keep association between the DOM and the entry and so the only way to associate is by position.

ng-repeat='i in [1,1,1] track by $index' will do what you want.

Closing the issue.

@mhevery mhevery closed this Apr 25, 2013

@IgorMinar

This comment has been minimized.

Show comment
Hide comment
@IgorMinar

IgorMinar Apr 25, 2013

Member

+1 for misko's reply

Member

IgorMinar commented Apr 25, 2013

+1 for misko's reply

@spamdaemon

This comment has been minimized.

Show comment
Hide comment
@spamdaemon

spamdaemon Apr 25, 2013

Ok that works great.I did not see this in the documentation.

spamdaemon commented Apr 25, 2013

Ok that works great.I did not see this in the documentation.

@spamdaemon spamdaemon deleted the spamdaemon:ngRepeatAllowDuplicates branch Apr 25, 2013

@aaronshaf

This comment has been minimized.

Show comment
Hide comment
@aaronshaf

aaronshaf May 23, 2013

Please put this in the in the documentation:

http://docs.angularjs.org/api/ng.directive:ngRepeat

aaronshaf commented May 23, 2013

Please put this in the in the documentation:

http://docs.angularjs.org/api/ng.directive:ngRepeat

@zakdances

This comment has been minimized.

Show comment
Hide comment
@zakdances

zakdances Jun 2, 2013

When I use @mhevery's example, all I get is

Syntax Error: Token 'track' is an unexpected token at column 10 of the expression [[entries]track by $index] starting at [track by $index].

This is my html attribute:

ng-repeat="entry in [entries]track by $index"

zakdances commented Jun 2, 2013

When I use @mhevery's example, all I get is

Syntax Error: Token 'track' is an unexpected token at column 10 of the expression [[entries]track by $index] starting at [track by $index].

This is my html attribute:

ng-repeat="entry in [entries]track by $index"
@KevinBrogan

This comment has been minimized.

Show comment
Hide comment
@KevinBrogan

KevinBrogan Jun 10, 2013

Contributor

@zakdances you're missing a space between [entries] and track

Contributor

KevinBrogan commented Jun 10, 2013

@zakdances you're missing a space between [entries] and track

@estufa8

This comment has been minimized.

Show comment
Hide comment
@estufa8

estufa8 Jun 29, 2013

same problem occurs using (key,value) with duplicated values, like in
ng-repeat="(k,v) in {a:0, b:0}"
track by $index does the trick, thanks

estufa8 commented Jun 29, 2013

same problem occurs using (key,value) with duplicated values, like in
ng-repeat="(k,v) in {a:0, b:0}"
track by $index does the trick, thanks

@nickretallack

This comment has been minimized.

Show comment
Hide comment
@nickretallack

nickretallack Jul 2, 2013

Why is this still undocumented?

nickretallack commented Jul 2, 2013

Why is this still undocumented?

@guilloche

This comment has been minimized.

Show comment
Hide comment
@guilloche

guilloche Jul 7, 2013

Please add it to the documentation.

guilloche commented Jul 7, 2013

Please add it to the documentation.

@pkozlowski-opensource

This comment has been minimized.

Show comment
Hide comment
@pkozlowski-opensource

pkozlowski-opensource Jul 7, 2013

Member

@nickretallack @guilloche It is documented:
http://code.angularjs.org/1.1.5/docs/api/ng.directive:ngRepeat

Docs at the http://docs.angularjs.org/ point to the latest stable version (1.0.x). If you need docs for the unstable branch (1.1.x) go to http://code.angularjs.org/

Member

pkozlowski-opensource commented Jul 7, 2013

@nickretallack @guilloche It is documented:
http://code.angularjs.org/1.1.5/docs/api/ng.directive:ngRepeat

Docs at the http://docs.angularjs.org/ point to the latest stable version (1.0.x). If you need docs for the unstable branch (1.1.x) go to http://code.angularjs.org/

@guilloche

This comment has been minimized.

Show comment
Hide comment
@guilloche

guilloche Jul 7, 2013

Thanks a lot for the explanation.

guilloche commented Jul 7, 2013

Thanks a lot for the explanation.

@tamtakoe

This comment has been minimized.

Show comment
Hide comment
@tamtakoe

tamtakoe Jul 31, 2013

I want use ng-repeat without duplicates. This behaviour is prefer for me, but I dont want see error message. Can I disable it?

What about perfomance when ng-repeat delete duplicates?

tamtakoe commented Jul 31, 2013

I want use ng-repeat without duplicates. This behaviour is prefer for me, but I dont want see error message. Can I disable it?

What about perfomance when ng-repeat delete duplicates?

@netpoetica

This comment has been minimized.

Show comment
Hide comment
@netpoetica

netpoetica Aug 8, 2013

Contributor

This is a major headache, really. Even the documentation in the new version simply does not make any sense. A code example for this text example would be helpful.

http://code.angularjs.org/1.1.5/docs/api/ng.directive:ngRepeat

"For example: item in items track by $id(item). A built in $id() function can be used to assign a unique $$hashKey property to each item in the array. This property is then used as a key to associated DOM elements with the corresponding item in the array by identity. Moving the same object in array would move the DOM element in the same way ian the DOM."

What is "$id" supposed to be? It's a "function" but with a name of an internal Angular item "$id", so it looks like $id is a provided helper method, but I doubt it. So I'm imaginging maybe this is a $scope.$id = function(){//do it yourself, we dont care }); type of thing, but due to the fact that 1 sentence into the documentation, I already don't know what they're trying to do, going any further is simply impossible - at least for me anyway. Maybe it's just me.

More importantly, this sounds like some kind of problem that someone on the Angular dev team would understand really well because they've fixed bugs on it, they understand how it effects core tracking mechanisms. However, someone who doesn't have that level of detailed understanding, has absolutely no idea what problem you are trying to solve by making it impossible to do something that makes so much sense.

Contributor

netpoetica commented Aug 8, 2013

This is a major headache, really. Even the documentation in the new version simply does not make any sense. A code example for this text example would be helpful.

http://code.angularjs.org/1.1.5/docs/api/ng.directive:ngRepeat

"For example: item in items track by $id(item). A built in $id() function can be used to assign a unique $$hashKey property to each item in the array. This property is then used as a key to associated DOM elements with the corresponding item in the array by identity. Moving the same object in array would move the DOM element in the same way ian the DOM."

What is "$id" supposed to be? It's a "function" but with a name of an internal Angular item "$id", so it looks like $id is a provided helper method, but I doubt it. So I'm imaginging maybe this is a $scope.$id = function(){//do it yourself, we dont care }); type of thing, but due to the fact that 1 sentence into the documentation, I already don't know what they're trying to do, going any further is simply impossible - at least for me anyway. Maybe it's just me.

More importantly, this sounds like some kind of problem that someone on the Angular dev team would understand really well because they've fixed bugs on it, they understand how it effects core tracking mechanisms. However, someone who doesn't have that level of detailed understanding, has absolutely no idea what problem you are trying to solve by making it impossible to do something that makes so much sense.

@netpoetica

This comment has been minimized.

Show comment
Hide comment
@netpoetica

netpoetica Aug 8, 2013

Contributor

Maybe I am doing something wrong, but see this Plunkr for an example of why this duplicates maybe should be allowed: http://plnkr.co/edit/km4uoNPRjh624QHgEeeB?p=preview

This is the result of implementing from the docs, "track as $id($index)". If you click "add" to add a new item to your array, and then select an item, and then click "add" again - BOOM, both items reset. Why? - if you remove the "track by" from the Stuff select, you will get duplicates error.

It seems to me like Angular doesn't like that the select list has the same option values in both ng-options within the ng-repeat. As you can see there, the important thing is that Angular knows to set the value of my $scope view model property to the selected option - the select options should not even be tracked by angular - they do not change. The selected state changes, but that is it.

I will post this to the Google group as well for an answer - I am posting here to illustrate a point, which is mostly the point that I have spent the last 2 days trying to wrap my ahead around a silly error that looks, again, like maybe the angular team is aware of why we "shouldn't" have duplicates, but it certainly doesn't make sense to me - and quite a few other people if you do a search for "Duplicates not allowed"

See Google group post: https://groups.google.com/forum/#!topic/angular/Biy9jjsyHNM

Much confusion around this topic. Some people are seeing results with track by $id($index) but I think my above example shows that this issue is not solved.

EDIT: Trying to bind to the item in the loop seems to create a closure issue where all items bind to the same model, even though technically it should be a different reference in every iteration. If you instead specify to bind ng-model to the parent element by index: http://plnkr.co/edit/1CmulrjigYQYjmfewUcJ?p=preview

Contributor

netpoetica commented Aug 8, 2013

Maybe I am doing something wrong, but see this Plunkr for an example of why this duplicates maybe should be allowed: http://plnkr.co/edit/km4uoNPRjh624QHgEeeB?p=preview

This is the result of implementing from the docs, "track as $id($index)". If you click "add" to add a new item to your array, and then select an item, and then click "add" again - BOOM, both items reset. Why? - if you remove the "track by" from the Stuff select, you will get duplicates error.

It seems to me like Angular doesn't like that the select list has the same option values in both ng-options within the ng-repeat. As you can see there, the important thing is that Angular knows to set the value of my $scope view model property to the selected option - the select options should not even be tracked by angular - they do not change. The selected state changes, but that is it.

I will post this to the Google group as well for an answer - I am posting here to illustrate a point, which is mostly the point that I have spent the last 2 days trying to wrap my ahead around a silly error that looks, again, like maybe the angular team is aware of why we "shouldn't" have duplicates, but it certainly doesn't make sense to me - and quite a few other people if you do a search for "Duplicates not allowed"

See Google group post: https://groups.google.com/forum/#!topic/angular/Biy9jjsyHNM

Much confusion around this topic. Some people are seeing results with track by $id($index) but I think my above example shows that this issue is not solved.

EDIT: Trying to bind to the item in the loop seems to create a closure issue where all items bind to the same model, even though technically it should be a different reference in every iteration. If you instead specify to bind ng-model to the parent element by index: http://plnkr.co/edit/1CmulrjigYQYjmfewUcJ?p=preview

@nickretallack

This comment has been minimized.

Show comment
Hide comment
@nickretallack

nickretallack Aug 9, 2013

In the current stable version of angular you can put duplicate items in an ng-repeat just fine. Why did we break this feature?

<div ng-app><div ng-repeat="item in [1,1,1]">{{item}}</div></div>

I'm afraid if this breaking change ever goes into the stable version it is going to cause unexpected problems for people.

Imagine this situation: A developer writes an app with an ng-repeat that usually doesn't contain duplicates. However, in rare situations, it does. They release their web app. Later, users report that lists disappear from the page sometimes. Imagine being this developer and wondering what went wrong.

nickretallack commented Aug 9, 2013

In the current stable version of angular you can put duplicate items in an ng-repeat just fine. Why did we break this feature?

<div ng-app><div ng-repeat="item in [1,1,1]">{{item}}</div></div>

I'm afraid if this breaking change ever goes into the stable version it is going to cause unexpected problems for people.

Imagine this situation: A developer writes an app with an ng-repeat that usually doesn't contain duplicates. However, in rare situations, it does. They release their web app. Later, users report that lists disappear from the page sometimes. Imagine being this developer and wondering what went wrong.

@mgan59

This comment has been minimized.

Show comment
Hide comment
@mgan59

mgan59 Aug 12, 2013

I just had a painful hour trying to figure out what was wrong. May want to get the current docs - http://docs.angularjs.org/api/ng.directive:ngRepeat updated for those of us that are new to angular.

SMASH

mgan59 commented Aug 12, 2013

I just had a painful hour trying to figure out what was wrong. May want to get the current docs - http://docs.angularjs.org/api/ng.directive:ngRepeat updated for those of us that are new to angular.

SMASH

alexey-pelykh pushed a commit to osmandapp/OsmAnd-external-skia that referenced this pull request Aug 21, 2014

epoger@google.com
rebaseline_server: tiny fix to un-break UI when ng-repeate (key, valu…
…e) has duplicate values

See angular/angular.js#2505

(SkipBuildbotRuns)
TBR=jcgregorio

Review URL: https://codereview.chromium.org/46063005

git-svn-id: http://skia.googlecode.com/svn/trunk@11972 2bbb7eff-a529-9590-31e7-b0007b416f81

jrab89 added a commit to jrab89/angular_examples that referenced this pull request Apr 2, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment