feat(ngRepeat): expose the iterating properties over the alias #9622

lgalfaso · 2014-10-14T19:16:45Z

Expose the properties $index, $first, $middle, $last, $odd and $even as part of the alias

caitp · 2014-10-14T19:19:28Z

src/ng/directive/ngRepeat.js

@@ -328,6 +330,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {

          if (aliasAs) {
            $scope[aliasAs] = collection;
+            LocalAlias.prototype = collection;


I don't think this is a smart idea

I know you're thinking "oh this is really clever because this way they can get the collection properties the same way", but it's not --- it can potentially shadow properties, which would be bad --- and it will make accessing collection properties this way slower.

Lets not do this.

I understand this, but I do not want to break https://github.com/angular/angular.js/pull/9622/files#diff-360c2ce98ed0ad852cb86146eae935f3R523

we can't break it, but I don't think we should make it terrible either. Either create a new name for the property, or extend the microsyntax (bad), or come up with something else for this.

I do not want to extend the microsyntax... I think there is already a lot going on there and more options will not make it simpler. An alternative would be something based on the alias... say create an object named as alias with a $ prepended (and for this object to have the properties)?

we could do ${{userSpecifiedAliasName}}.$index/etc, I guess

lgalfaso · 2014-10-14T19:45:54Z

Updated the PR so the properties are exposed as a new object named ${{userSpecifiedAliasName}}

caitp · 2014-10-14T19:51:47Z

src/ng/directive/ngRepeat.js

@@ -212,17 +212,19 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
  var NG_REMOVED = '$$NG_REMOVED';
  var ngRepeatMinErr = minErr('ngRepeat');

-  var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength) {
+  var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength, aliasAs, LocalAlias) {
+    var aliasRef = aliasAs ? new LocalAlias() : {};


you know, if there's no aliasAs expression, and we aren't going to expose this object, why not just declare the object outside of the closure, so that we don't keep re-creating it?

you are right, I think it is better. Updated the PR with this change

Expose the properties `$index`, `$first`, `$middle`, `$last`, `$odd` and `$even` as part of the alias Closes angular#9582

caitp · 2014-10-14T20:45:06Z

test/ng/directive/ngRepeatSpec.js

@@ -492,6 +492,43 @@ describe('ngRepeat', function() {
        return text.replace(/[\-\[\]\/\{\}\(\)\*\+\?\.\\\^\$\|]/g, "\\$&");
      }
    }));
+
+    it('should expose the index properties using `${{userSpecifiedAliasName}}`', inject(function() {
+      var expression = '{{item}}-{{$results.' + ['$index', '$first', '$middle', '$last', '$odd', '$even'].join('}}-{{$results.') + '}}:';


I'm kinda torn on this --- I know I said it's better than the other thing, but I'm not sure... it sucks this way too =\

I think I need someone else to weigh in on which way would be better. I don't like potentially shadowing object keys, and I don't like the slight impact of crawling up the prototype chain. But maybe it's better than this

I like the original idea of reusing the same alias name, the collection is an array and that makes it somehow less problematic in regards to shadowing properties (still not great, but not really bad)
People that need to use this are a very small % of the uses of ng-repeat so I am not too worried about a small performance impact

I don't like it --- I think the extra directive option is better, tbh

you mean to extend the microsyntax or to use `${{aliasName}}?

neither. I think just using a new directive for setting up the alias is better

I don't think it's too bad --- I suggested earlier that we could set up a dictionary of aliases on $scope (which would be touched by the dictionary) to keep them in sync with assign expressions --- then we'd use assign rather than = to set up the $index/etc vars, and do the hard work without touching dirty checking.

Oh, sorry, maybe I misunderstood your idea on ng-alias, I thought that the idea was that if you have ng-alias='foo as bar', then if you modify foo, then bar would reflect that change and viceversa (of course, everything with $parse and expressions). Now, when on top of this you add one-time binding, this is when things get non-trivial. I am not saying impossible, just that need to be done with a lot of care

one-time bindings would not make sense for an alias, because it would be completely unrelated to change detection (in my vision of it) --- so a one time binding would not matter, and that's fine

one-time binding is unrelated, but an implementation of ng-alias should be able to handle ng-alias='["foo"] as foo'

No it shouldn't, ["foo"] is not a binding. We should be dealing with identifiers --- assignable expressions only

lgalfaso · 2015-01-31T19:40:21Z

there is no traction on this idea, closing

caitp reviewed Oct 14, 2014
View reviewed changes

lgalfaso force-pushed the ngrepeat-alias-with-properties branch from 27662bf to d8fc6ca Compare October 14, 2014 19:23

caitp reviewed Oct 14, 2014
View reviewed changes

caitp added type: feature component: ngRepeat frequency: low labels Oct 14, 2014

caitp added this to the Backlog milestone Oct 14, 2014

mary-poppins added cla: yes and removed cla: no labels Oct 14, 2014

feat(ngRepeat): expose the iterating properties over the alias

d71d689

Expose the properties `$index`, `$first`, `$middle`, `$last`, `$odd` and `$even` as part of the alias Closes angular#9582

lgalfaso force-pushed the ngrepeat-alias-with-properties branch from d8fc6ca to d71d689 Compare October 14, 2014 20:13

caitp reviewed Oct 14, 2014
View reviewed changes

petebacondarwin force-pushed the master branch from 4dd5a20 to 998c61c Compare October 19, 2014 10:58

lgalfaso closed this Jan 31, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ngRepeat): expose the iterating properties over the alias #9622

feat(ngRepeat): expose the iterating properties over the alias #9622

lgalfaso commented Oct 14, 2014

caitp Oct 14, 2014

caitp Oct 14, 2014

lgalfaso Oct 14, 2014

caitp Oct 14, 2014

lgalfaso Oct 14, 2014

caitp Oct 14, 2014

lgalfaso commented Oct 14, 2014

caitp Oct 14, 2014

lgalfaso Oct 14, 2014

caitp Oct 14, 2014

lgalfaso Oct 15, 2014

caitp Oct 15, 2014

lgalfaso Oct 15, 2014

caitp Oct 15, 2014

caitp Oct 15, 2014

lgalfaso Oct 15, 2014

caitp Oct 15, 2014

lgalfaso Oct 15, 2014

caitp Oct 15, 2014

lgalfaso commented Jan 31, 2015

feat(ngRepeat): expose the iterating properties over the alias #9622

feat(ngRepeat): expose the iterating properties over the alias #9622

Conversation

lgalfaso commented Oct 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgalfaso commented Oct 14, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgalfaso commented Jan 31, 2015