List watch should report initial content as additions

[vicb]

relates to #751

When a list is watch, the first reported CollectionChangeRecord contains the list in its iterable field and the additions is null.

We should change this for the additions to report all ements as added [null -> <index>].

It could allow simplifying some code by making the first iteration a common case rather than a special case.

[vicb]

TODO

When this is solved, update ngRepeat and ngClass

[vicb]

@mhevery (or whoever can help me)

I have investigated this a little bit more.

The problem is in

  Watch watch(AST expression, ReactionFn reactionFn) {
    WatchRecord<_Handler> watchRecord =
        _cache.putIfAbsent(expression.expression,
            () => expression.setupWatch(this));
    return watchRecord.handler.addReactionFn(reactionFn);
  }

When watching a collection addCollectionWatch would call collectionHandler.acceptValue(astWR.currentValue); but at that time the reactionFn is not registered yet - see the last line of watch() above.

Then we'll only get notified on the second check() which is why additions are not reported.

I'm unsure of to fix this as the behavior should be specific for collections and I'm afraid my changes would have side effects.

Any hint on the best solution ?

[chirayuk]

I looked into this issue.  A fix would require a refactor of the way we do dirty checking for collections and maps.  In the general case, _CollectionChangeRecord should always have the changes since the reported change (i.e. it cannot call _reset() in _check())  (Not that the fix for simple cases is easy (e.g. when the watches for the collection are all set up in the same cycle) but if one were to call scope.watch() on an already watched collection expression, the reaction function for that will likely still be called without the additions/deletions if the list isn't changing since any intermediate _check() call would have called _reset() on the record.)  So one would have to handle the first change fired without the deltas.

Closing for now (but I'll keep this in mind during refactors.)

[vicb]

@chirayuk thanks for the info.

I'll keep this in mind during refactors

Do you have any specific things in mind ? If so i would to hear more about that.

[vicb]

Reopening as the behavior might change with the "unified" change detection (#1348)