From a10c02cb4107624b58b4b3f54da6bf0e0e70c6c6 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 17 Feb 2016 09:17:29 -0800 Subject: [PATCH] feat(iterable_differ): support immutable lists Closes #7127 --- .../differs/default_iterable_differ.ts | 36 ++++++++++--------- modules/angular2/src/facade/collection.dart | 9 ++++- modules/angular2/src/facade/collection.ts | 7 ++++ .../differs/default_iterable_differ_spec.ts | 31 +++++++++++++++- 4 files changed, 64 insertions(+), 19 deletions(-) diff --git a/modules/angular2/src/core/change_detection/differs/default_iterable_differ.ts b/modules/angular2/src/core/change_detection/differs/default_iterable_differ.ts index 878f5f08ab..12cb945502 100644 --- a/modules/angular2/src/core/change_detection/differs/default_iterable_differ.ts +++ b/modules/angular2/src/core/change_detection/differs/default_iterable_differ.ts @@ -109,7 +109,6 @@ export class DefaultIterableDiffer implements IterableDiffer { onDestroy() {} - // todo(vicb): optim for UnmodifiableListView (frozen arrays) check(collection: any): boolean { this._reset(); @@ -119,24 +118,27 @@ export class DefaultIterableDiffer implements IterableDiffer { var item; var itemTrackBy; if (isArray(collection)) { - var list = collection; - this._length = collection.length; + if (collection !== this._collection || !ListWrapper.isImmutable(collection)) { + var list = collection; + this._length = collection.length; - for (index = 0; index < this._length; index++) { - item = list[index]; - itemTrackBy = this._trackByFn(index, item); - if (record === null || !looseIdentical(record.trackById, itemTrackBy)) { - record = this._mismatch(record, item, itemTrackBy, index); - mayBeDirty = true; - } else { - if (mayBeDirty) { - // TODO(misko): can we limit this to duplicates only? - record = this._verifyReinsertion(record, item, itemTrackBy, index); + for (index = 0; index < this._length; index++) { + item = list[index]; + itemTrackBy = this._trackByFn(index, item); + if (record === null || !looseIdentical(record.trackById, itemTrackBy)) { + record = this._mismatch(record, item, itemTrackBy, index); + mayBeDirty = true; + } else { + if (mayBeDirty) { + // TODO(misko): can we limit this to duplicates only? + record = this._verifyReinsertion(record, item, itemTrackBy, index); + } + if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); } - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); - } - record = record._next; + record = record._next; + } + this._truncate(record); } } else { index = 0; @@ -156,9 +158,9 @@ export class DefaultIterableDiffer implements IterableDiffer { index++; }); this._length = index; + this._truncate(record); } - this._truncate(record); this._collection = collection; return this.isDirty; } diff --git a/modules/angular2/src/facade/collection.dart b/modules/angular2/src/facade/collection.dart index ecc54a6d17..3bf0eb1e52 100644 --- a/modules/angular2/src/facade/collection.dart +++ b/modules/angular2/src/facade/collection.dart @@ -1,6 +1,6 @@ library facade.collection; -import 'dart:collection' show IterableBase; +import 'dart:collection' show IterableBase, UnmodifiableListView; import 'dart:convert' show JsonEncoder; export 'dart:core' show Iterator, Map, List, Set; import 'dart:math' show max, min; @@ -106,6 +106,9 @@ class ListWrapper { static List createFixedSize(int size) => new List(size); static List createGrowableSize(int size) => new List.generate(size, (_) => null, growable: true); + static UnmodifiableListView createImmutable(List input) { + return new UnmodifiableListView(input); + } static bool contains(List m, k) => m.contains(k); static int indexOf(List list, value, [int startIndex = 0]) => @@ -221,6 +224,10 @@ class ListWrapper { } return solution; } + + static bool isImmutable(List l) { + return l is UnmodifiableListView; + } } bool isListLikeIterable(obj) => obj is Iterable; diff --git a/modules/angular2/src/facade/collection.ts b/modules/angular2/src/facade/collection.ts index ebc97f2eb6..f4c3f2752a 100644 --- a/modules/angular2/src/facade/collection.ts +++ b/modules/angular2/src/facade/collection.ts @@ -178,6 +178,11 @@ export class ListWrapper { static createFixedSize(size: number): any[] { return new Array(size); } static createGrowableSize(size: number): any[] { return new Array(size); } static clone(array: T[]): T[] { return array.slice(0); } + static createImmutable(array: T[]): T[] { + var result = ListWrapper.clone(array); + Object.seal(result); + return result; + } static forEachWithIndex(array: T[], fn: (t: T, n: number) => void) { for (var i = 0; i < array.length; i++) { fn(array[i], i); @@ -265,6 +270,8 @@ export class ListWrapper { } return solution; } + + static isImmutable(list: any[]): boolean { return Object.isSealed(list); } } export function isListLikeIterable(obj: any): boolean { diff --git a/modules/angular2/test/core/change_detection/differs/default_iterable_differ_spec.ts b/modules/angular2/test/core/change_detection/differs/default_iterable_differ_spec.ts index 226c7b1fa3..0d469119f9 100644 --- a/modules/angular2/test/core/change_detection/differs/default_iterable_differ_spec.ts +++ b/modules/angular2/test/core/change_detection/differs/default_iterable_differ_spec.ts @@ -31,7 +31,6 @@ class ComplexItem { toString() { return `{id: ${this.id}, color: ${this.color}}` } } -// todo(vicb): UnmodifiableListView / frozen object when implemented export function main() { describe('iterable differ', function() { describe('DefaultIterableDiffer', function() { @@ -314,6 +313,36 @@ export function main() { })); }); + it('should not diff immutable collections if they are the same', () => { + // Note: Use trackBy to know if diffing happened + var trackByCount = 0; + var trackBy = (index: number, item: any): any => { + trackByCount++; + return item; + }; + var differ = new DefaultIterableDiffer(trackBy); + var l1 = ListWrapper.createImmutable([1]); + + differ.check(l1); + expect(trackByCount).toBe(1); + expect(differ.toString()) + .toEqual( + iterableChangesAsString({collection: ['1[null->0]'], additions: ['1[null->0]']})); + + + trackByCount = 0; + differ.check(l1); + expect(trackByCount).toBe(0); + expect(differ.toString()) + .toEqual(iterableChangesAsString({collection: ['1'], previous: ['1']})); + + trackByCount = 0; + differ.check(l1); + expect(trackByCount).toBe(0); + expect(differ.toString()) + .toEqual(iterableChangesAsString({collection: ['1'], previous: ['1']})); + }); + describe('diff', () => { it('should return self when there is a change', () => { expect(differ.diff(['a', 'b'])).toBe(differ); });