From d24df799d30236c07c52646d7c9028f18a60dedd Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Tue, 12 Apr 2016 08:02:59 -0700 Subject: [PATCH] Revert "feat(iterable_differ): support immutable lists" In Dart, ImmutableLists are just a projection of an underlying list. I.e. if the underlying list changes, the ImmutableList also changes. So we can't make optimizations based on checking whether a collection is an ImmutableList. This reverts commit a10c02cb4107624b58b4b3f54da6bf0e0e70c6c6. Closes #8023 --- .../differs/default_iterable_differ.ts | 36 +++++++++---------- modules/angular2/src/facade/collection.dart | 9 +---- modules/angular2/src/facade/collection.ts | 6 ---- .../differs/default_iterable_differ_spec.ts | 31 +--------------- 4 files changed, 19 insertions(+), 63 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 12cb945502..878f5f08ab 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,6 +109,7 @@ export class DefaultIterableDiffer implements IterableDiffer { onDestroy() {} + // todo(vicb): optim for UnmodifiableListView (frozen arrays) check(collection: any): boolean { this._reset(); @@ -118,27 +119,24 @@ export class DefaultIterableDiffer implements IterableDiffer { var item; var itemTrackBy; if (isArray(collection)) { - if (collection !== this._collection || !ListWrapper.isImmutable(collection)) { - var list = collection; - this._length = collection.length; + 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); - } - if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); + 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); } - - record = record._next; + if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item); } - this._truncate(record); + + record = record._next; } } else { index = 0; @@ -158,9 +156,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 5768c09af1..3f5ec99c8a 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, UnmodifiableListView; +import 'dart:collection' show IterableBase; import 'dart:convert' show JsonEncoder; export 'dart:core' show Iterator, Map, List, Set; import 'dart:math' show max, min; @@ -110,9 +110,6 @@ 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]) => @@ -229,10 +226,6 @@ class ListWrapper { return solution; } - static bool isImmutable(List l) { - return l is UnmodifiableListView; - } - static List flatten(List l) { final res = []; l.forEach((item) { diff --git a/modules/angular2/src/facade/collection.ts b/modules/angular2/src/facade/collection.ts index 636bd4657d..936a196801 100644 --- a/modules/angular2/src/facade/collection.ts +++ b/modules/angular2/src/facade/collection.ts @@ -184,11 +184,6 @@ 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); @@ -277,7 +272,6 @@ export class ListWrapper { return solution; } - static isImmutable(list: any[]): boolean { return Object.isSealed(list); } static flatten(array: T[][]): T[] { let res = []; array.forEach((a) => res = res.concat(a)); 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 1668d114a0..d897999bf8 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,6 +31,7 @@ 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() { @@ -313,36 +314,6 @@ 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); });