Allow Map.values in ng-repeat

[achatham]

Often the natural way to store data is in a Map, and I'd like to use Map.values in an ng-repeat. ng-repeat has support for Iterable, but HashMap.values creates a new object on each call, so it fails the identical() check used in $watch. This leads to a $digest loop. $watch should be robust to this.

If this isn't possible, it would be nice if $digest loops noticed that they were caused by the underlying collection changing object ids and printed a less confusing error.

[jbdeboer]

See also #394

The best fix for this would be a stable Map.values object. Could you fill a dartbug.com?

[achatham]

It's not clear to me that this is a problem with dart. Most code wouldn't
have a problem with this, and it's a pretty specific requirement that the
object id of Map.values is stable.

On Wed, Jan 8, 2014 at 10:50 AM, James deBoer [email protected]:

See also #394 #394

The best fix for this would be a stable Map.values object. Could you fill
a dartbug.com?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/397#issuecomment-31865055
.

[achatham]

BTW, you run into a similar problem when you do:

List get values => [0, 1, 2];

because that creates a new object id on each access.

On Wed, Jan 8, 2014 at 10:54 AM, Andrew Chatham [email protected] wrote:

It's not clear to me that this is a problem with dart. Most code wouldn't
have a problem with this, and it's a pretty specific requirement that the
object id of Map.values is stable.

On Wed, Jan 8, 2014 at 10:50 AM, James deBoer [email protected]:

See also #394 #394

The best fix for this would be a stable Map.values object. Could you fill
a dartbug.com?

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/397#issuecomment-31865055
.

[mhevery]

This will be fixed with the new change detection.

[chalin]

#605, and a little tweak to #763, will add support for all Map properties including Map.values and Map.keys.

[mhevery]

My latest thinking on this is that this is a two part fix.

PART1:
Do not treat Map as special and expect that dereferencing map is always done through [] operator as in map['key']. This way map['length'] and map.length can easily be told apart and no special casing is needed. Since scope context objects are maps this would make it very unnatural to write code in templates since {{ctrl.foo}} would have to become {{this['ctrl'].foo}}. See part 2.

PART2:
Stop using Map for Scope context and instead use the component controller. This means that we will not have to publish the controller using publishAs:"ctrl". This means that {{ctrl.foo}} (or if Part1 is applied then {{this['ctrl'].foo}}) becomes just {{foo}}.

Together Part1 and Part2: create a clean solution to this problem.

[vicb]

@mhevery with your part2, how would you access a foo in the root scope ?

[chalin]

Or in an ancestor scope?

[chalin]

As part of adding support for map.keys, map.values, etc, without special treatment, I have a proposal: please see #763. As stated in the comment. If you are in agreement with the approach, the PR can be adapted (by adding a single extra test), to support map.keys and map.values without causing digest destabilization. (If #605 is accepted, then that gives us support for map.keys and map.values, etc right away :). Solving issues #359, #394, #397 (this one) and #608.

[chalin]

I believe that PART2 can be accomplished without a change in syntax: have the context be of a type other than map, say Context, and give this type special treatment in the evaluation and change detaction rules. This is how maps get support for both the .field and [ ] syntaxes --- namely, by testing the type of the receiver. E.g., from core/parser/eval_access.dart and change_detection/dirty_checking_change_detector.dart:

    if (obj is Map) {
        _mode =  _MODE_MAP_FIELD_;
        _instanceMirror = null;
      } else if ...

would become obj is Context.

[chalin]

@mhevery, I would be willing to attempt to implement PART2 along the lines of what I outlined above, if you believe that this would be a useful contribution that no one else on the team is working on.

[mhevery]

Ref: #394

[vicb]

fixed in #1269