From cee2318110eeea115e5f6fc5bfc814cbaa7d90d8 Mon Sep 17 00:00:00 2001 From: Kara Erickson Date: Mon, 1 Feb 2016 18:31:26 -0800 Subject: [PATCH] feat(ngFor): add custom trackBy function support Make it possible to track items in iterables in custom ways (e.g. by ID or index), rather than simply by identity. Closes #6779 --- .../angular2/src/common/directives/ng_for.ts | 10 +- .../directives/observable_list_diff.dart | 2 +- modules/angular2/src/core/change_detection.ts | 3 +- .../core/change_detection/change_detection.ts | 9 +- .../differs/default_iterable_differ.ts | 104 +++++++++--------- .../differs/iterable_differs.ts | 9 +- .../test/common/directives/ng_for_spec.ts | 28 ++++- .../differs/default_iterable_differ_spec.ts | 84 +++++++++++++- modules/angular2/test/public_api_spec.ts | 1 + tools/public_api_guard/public_api_spec.ts | 4 +- 10 files changed, 192 insertions(+), 62 deletions(-) diff --git a/modules/angular2/src/common/directives/ng_for.ts b/modules/angular2/src/common/directives/ng_for.ts index ec768c4981..ea63804d34 100644 --- a/modules/angular2/src/common/directives/ng_for.ts +++ b/modules/angular2/src/common/directives/ng_for.ts @@ -6,7 +6,8 @@ import { IterableDiffers, ViewContainerRef, TemplateRef, - EmbeddedViewRef + EmbeddedViewRef, + TrackByFn } from 'angular2/core'; import {isPresent, isBlank} from 'angular2/src/facade/lang'; @@ -59,10 +60,11 @@ import {isPresent, isBlank} from 'angular2/src/facade/lang'; * See a [live demo](http://plnkr.co/edit/KVuXxDp0qinGDyo307QW?p=preview) for a more detailed * example. */ -@Directive({selector: '[ngFor][ngForOf]', inputs: ['ngForOf', 'ngForTemplate']}) +@Directive({selector: '[ngFor][ngForOf]', inputs: ['ngForTrackBy', 'ngForOf', 'ngForTemplate']}) export class NgFor implements DoCheck { /** @internal */ _ngForOf: any; + _ngForTrackBy: TrackByFn; private _differ: IterableDiffer; constructor(private _viewContainer: ViewContainerRef, private _templateRef: TemplateRef, @@ -71,7 +73,7 @@ export class NgFor implements DoCheck { set ngForOf(value: any) { this._ngForOf = value; if (isBlank(this._differ) && isPresent(value)) { - this._differ = this._iterableDiffers.find(value).create(this._cdr); + this._differ = this._iterableDiffers.find(value).create(this._cdr, this._ngForTrackBy); } } @@ -81,6 +83,8 @@ export class NgFor implements DoCheck { } } + set ngForTrackBy(value: TrackByFn) { this._ngForTrackBy = value; } + ngDoCheck() { if (isPresent(this._differ)) { var changes = this._differ.diff(this._ngForOf); diff --git a/modules/angular2/src/common/directives/observable_list_diff.dart b/modules/angular2/src/common/directives/observable_list_diff.dart index 034dd66375..eb3e929532 100644 --- a/modules/angular2/src/common/directives/observable_list_diff.dart +++ b/modules/angular2/src/common/directives/observable_list_diff.dart @@ -57,7 +57,7 @@ class ObservableListDiff extends DefaultIterableDiffer { class ObservableListDiffFactory implements IterableDifferFactory { const ObservableListDiffFactory(); bool supports(obj) => obj is ObservableList; - IterableDiffer create(ChangeDetectorRef cdRef) { + IterableDiffer create(ChangeDetectorRef cdRef, [Function trackByFn]) { return new ObservableListDiff(cdRef); } } diff --git a/modules/angular2/src/core/change_detection.ts b/modules/angular2/src/core/change_detection.ts index 6c732c33ea..1d81e799fb 100644 --- a/modules/angular2/src/core/change_detection.ts +++ b/modules/angular2/src/core/change_detection.ts @@ -20,5 +20,6 @@ export { IterableDifferFactory, KeyValueDiffers, KeyValueDiffer, - KeyValueDifferFactory + KeyValueDifferFactory, + TrackByFn } from './change_detection/change_detection'; diff --git a/modules/angular2/src/core/change_detection/change_detection.ts b/modules/angular2/src/core/change_detection/change_detection.ts index 2b8954243d..beec120241 100644 --- a/modules/angular2/src/core/change_detection/change_detection.ts +++ b/modules/angular2/src/core/change_detection/change_detection.ts @@ -1,4 +1,4 @@ -import {IterableDiffers, IterableDifferFactory} from './differs/iterable_differs'; +import {IterableDiffers, IterableDifferFactory, TrackByFn} from './differs/iterable_differs'; import {DefaultIterableDifferFactory} from './differs/default_iterable_differ'; import {KeyValueDiffers, KeyValueDifferFactory} from './differs/keyvalue_differs'; import {DefaultKeyValueDifferFactory} from './differs/default_keyvalue_differ'; @@ -37,7 +37,12 @@ export {BindingRecord, BindingTarget} from './binding_record'; export {DirectiveIndex, DirectiveRecord} from './directive_record'; export {DynamicChangeDetector} from './dynamic_change_detector'; export {ChangeDetectorRef} from './change_detector_ref'; -export {IterableDiffers, IterableDiffer, IterableDifferFactory} from './differs/iterable_differs'; +export { + IterableDiffers, + IterableDiffer, + IterableDifferFactory, + TrackByFn +} from './differs/iterable_differs'; export {KeyValueDiffers, KeyValueDiffer, KeyValueDifferFactory} from './differs/keyvalue_differs'; export {PipeTransform} from './pipe_transform'; export {WrappedValue, SimpleChange} from './change_detection_util'; 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 366c117c60..bf8e715f35 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 @@ -1,11 +1,6 @@ import {CONST} from 'angular2/src/facade/lang'; import {BaseException} from 'angular2/src/facade/exceptions'; -import { - isListLikeIterable, - iterateListLike, - ListWrapper, - MapWrapper -} from 'angular2/src/facade/collection'; +import {isListLikeIterable, iterateListLike, ListWrapper} from 'angular2/src/facade/collection'; import { isBlank, @@ -17,17 +12,21 @@ import { } from 'angular2/src/facade/lang'; import {ChangeDetectorRef} from '../change_detector_ref'; -import {IterableDiffer, IterableDifferFactory} from '../differs/iterable_differs'; +import {IterableDiffer, IterableDifferFactory, TrackByFn} from '../differs/iterable_differs'; @CONST() export class DefaultIterableDifferFactory implements IterableDifferFactory { supports(obj: Object): boolean { return isListLikeIterable(obj); } - create(cdRef: ChangeDetectorRef): DefaultIterableDiffer { return new DefaultIterableDiffer(); } + create(cdRef: ChangeDetectorRef, trackByFn?: TrackByFn): DefaultIterableDiffer { + return new DefaultIterableDiffer(trackByFn); + } } +var trackByIdentity = (index: number, item: any) => item; + export class DefaultIterableDiffer implements IterableDiffer { - private _collection = null; private _length: number = null; + private _collection = null; // Keeps track of the used records at any point in time (during & across `_check()` calls) private _linkedRecords: _DuplicateMap = null; // Keeps track of the removed records at any point in time during `_check()` calls. @@ -42,6 +41,10 @@ export class DefaultIterableDiffer implements IterableDiffer { private _removalsHead: CollectionChangeRecord = null; private _removalsTail: CollectionChangeRecord = null; + constructor(private _trackByFn?: TrackByFn) { + this._trackByFn = isPresent(this._trackByFn) ? this._trackByFn : trackByIdentity; + } + get collection() { return this._collection; } get length(): number { return this._length; } @@ -104,31 +107,37 @@ export class DefaultIterableDiffer implements IterableDiffer { var mayBeDirty: boolean = false; var index: number; var item; - + var itemTrackBy; if (isArray(collection)) { var list = collection; this._length = collection.length; for (index = 0; index < this._length; index++) { item = list[index]; - if (record === null || !looseIdentical(record.item, item)) { - record = this._mismatch(record, item, 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, index); + } else { + if (mayBeDirty) { + // TODO(misko): can we limit this to duplicates only? + record = this._verifyReinsertion(record, item, itemTrackBy, index); + } + record.item = item; } + record = record._next; } } else { index = 0; iterateListLike(collection, (item) => { - if (record === null || !looseIdentical(record.item, item)) { - record = this._mismatch(record, item, 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, index); + record = this._verifyReinsertion(record, item, itemTrackBy, index); } record = record._next; index++; @@ -190,7 +199,8 @@ export class DefaultIterableDiffer implements IterableDiffer { * * @internal */ - _mismatch(record: CollectionChangeRecord, item, index: number): CollectionChangeRecord { + _mismatch(record: CollectionChangeRecord, item: any, itemTrackBy: any, + index: number): CollectionChangeRecord { // The previous record after which we will append the current one. var previousRecord: CollectionChangeRecord; @@ -203,19 +213,20 @@ export class DefaultIterableDiffer implements IterableDiffer { } // Attempt to see if we have seen the item before. - record = this._linkedRecords === null ? null : this._linkedRecords.get(item, index); + record = this._linkedRecords === null ? null : this._linkedRecords.get(itemTrackBy, index); if (record !== null) { // We have seen this before, we need to move it forward in the collection. this._moveAfter(record, previousRecord, index); } else { // Never seen it, check evicted list. - record = this._unlinkedRecords === null ? null : this._unlinkedRecords.get(item); + record = this._unlinkedRecords === null ? null : this._unlinkedRecords.get(itemTrackBy); if (record !== null) { // It is an item which we have evicted earlier: reinsert it back into the list. this._reinsertAfter(record, previousRecord, index); } else { // It is a new item: add it. - record = this._addAfter(new CollectionChangeRecord(item), previousRecord, index); + record = + this._addAfter(new CollectionChangeRecord(item, itemTrackBy), previousRecord, index); } } return record; @@ -248,15 +259,17 @@ export class DefaultIterableDiffer implements IterableDiffer { * * @internal */ - _verifyReinsertion(record: CollectionChangeRecord, item, index: number): CollectionChangeRecord { + _verifyReinsertion(record: CollectionChangeRecord, item: any, itemTrackBy: any, + index: number): CollectionChangeRecord { var reinsertRecord: CollectionChangeRecord = - this._unlinkedRecords === null ? null : this._unlinkedRecords.get(item); + this._unlinkedRecords === null ? null : this._unlinkedRecords.get(itemTrackBy); if (reinsertRecord !== null) { record = this._reinsertAfter(reinsertRecord, record._prev, index); } else if (record.currentIndex != index) { record.currentIndex = index; this._addToMoves(record, index); } + record.item = item; return record; } @@ -457,31 +470,20 @@ export class DefaultIterableDiffer implements IterableDiffer { } toString(): string { - var record: CollectionChangeRecord; - var list = []; - for (record = this._itHead; record !== null; record = record._next) { - list.push(record); - } + this.forEachItem((record) => list.push(record)); var previous = []; - for (record = this._previousItHead; record !== null; record = record._nextPrevious) { - previous.push(record); - } + this.forEachPreviousItem((record) => previous.push(record)); var additions = []; - for (record = this._additionsHead; record !== null; record = record._nextAdded) { - additions.push(record); - } + this.forEachAddedItem((record) => additions.push(record)); + var moves = []; - for (record = this._movesHead; record !== null; record = record._nextMoved) { - moves.push(record); - } + this.forEachMovedItem((record) => moves.push(record)); var removals = []; - for (record = this._removalsHead; record !== null; record = record._nextRemoved) { - removals.push(record); - } + this.forEachRemovedItem((record) => removals.push(record)); return "collection: " + list.join(', ') + "\n" + "previous: " + previous.join(', ') + "\n" + "additions: " + additions.join(', ') + "\n" + "moves: " + moves.join(', ') + "\n" + @@ -512,7 +514,7 @@ export class CollectionChangeRecord { /** @internal */ _nextMoved: CollectionChangeRecord = null; - constructor(public item: any) {} + constructor(public item: any, public trackById: any) {} toString(): string { return this.previousIndex === this.currentIndex ? @@ -550,13 +552,13 @@ class _DuplicateItemRecordList { } } - // Returns a CollectionChangeRecord having CollectionChangeRecord.item == item and + // Returns a CollectionChangeRecord having CollectionChangeRecord.trackById == trackById and // CollectionChangeRecord.currentIndex >= afterIndex - get(item: any, afterIndex: number): CollectionChangeRecord { + get(trackById: any, afterIndex: number): CollectionChangeRecord { var record: CollectionChangeRecord; for (record = this._head; record !== null; record = record._nextDup) { if ((afterIndex === null || afterIndex < record.currentIndex) && - looseIdentical(record.item, item)) { + looseIdentical(record.trackById, trackById)) { return record; } } @@ -599,7 +601,7 @@ class _DuplicateMap { put(record: CollectionChangeRecord) { // todo(vicb) handle corner cases - var key = getMapKey(record.item); + var key = getMapKey(record.trackById); var duplicates = this.map.get(key); if (!isPresent(duplicates)) { @@ -610,17 +612,17 @@ class _DuplicateMap { } /** - * Retrieve the `value` using key. Because the CollectionChangeRecord value maybe one which we + * Retrieve the `value` using key. Because the CollectionChangeRecord value may be one which we * have already iterated over, we use the afterIndex to pretend it is not there. * * Use case: `[a, b, c, a, a]` if we are at index `3` which is the second `a` then asking if we * have any more `a`s needs to return the last `a` not the first or second. */ - get(value: any, afterIndex: number = null): CollectionChangeRecord { - var key = getMapKey(value); + get(trackById: any, afterIndex: number = null): CollectionChangeRecord { + var key = getMapKey(trackById); var recordList = this.map.get(key); - return isBlank(recordList) ? null : recordList.get(value, afterIndex); + return isBlank(recordList) ? null : recordList.get(trackById, afterIndex); } /** @@ -629,7 +631,7 @@ class _DuplicateMap { * The list of duplicates also is removed from the map if it gets empty. */ remove(record: CollectionChangeRecord): CollectionChangeRecord { - var key = getMapKey(record.item); + var key = getMapKey(record.trackById); // todo(vicb) // assert(this.map.containsKey(key)); var recordList: _DuplicateItemRecordList = this.map.get(key); diff --git a/modules/angular2/src/core/change_detection/differs/iterable_differs.ts b/modules/angular2/src/core/change_detection/differs/iterable_differs.ts index e9a173fe4d..fd3eb148d3 100644 --- a/modules/angular2/src/core/change_detection/differs/iterable_differs.ts +++ b/modules/angular2/src/core/change_detection/differs/iterable_differs.ts @@ -13,12 +13,19 @@ export interface IterableDiffer { onDestroy(); } +/** + * An optional function passed into {@link NgFor} that defines how to track + * items in an iterable (e.g. by index or id) + */ +export interface TrackByFn { (index: number, item: any): any; } + + /** * Provides a factory for {@link IterableDiffer}. */ export interface IterableDifferFactory { supports(objects: any): boolean; - create(cdRef: ChangeDetectorRef): IterableDiffer; + create(cdRef: ChangeDetectorRef, trackByFn?: TrackByFn): IterableDiffer; } /** diff --git a/modules/angular2/test/common/directives/ng_for_spec.ts b/modules/angular2/test/common/directives/ng_for_spec.ts index 64c39934bb..a56ae0296c 100644 --- a/modules/angular2/test/common/directives/ng_for_spec.ts +++ b/modules/angular2/test/common/directives/ng_for_spec.ts @@ -16,7 +16,7 @@ import { import {ListWrapper} from 'angular2/src/facade/collection'; import {Component, View, TemplateRef, ContentChild} from 'angular2/core'; import {NgFor} from 'angular2/src/common/directives/ng_for'; - +import {By} from 'angular2/platform/common_dom'; export function main() { describe('ngFor', () => { @@ -360,6 +360,31 @@ export function main() { async.done(); }); })); + + it('should use custom track by if function is provided', + inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => { + var template = + ``; + tcb.overrideTemplate(TestComponent, template) + .createAsync(TestComponent) + .then((fixture) => { + var buildItemList = + () => { + fixture.debugElement.componentInstance.items = [{'id': 'a'}]; + fixture.detectChanges(); + return fixture.debugElement.queryAll(By.css('p'))[0]; + } + + var firstP = buildItemList(); + var finalP = buildItemList(); + expect(finalP.nativeElement).toBe(firstP.nativeElement); + async.done(); + }); + })); + + }); } @@ -373,6 +398,7 @@ class TestComponent { @ContentChild(TemplateRef) contentTpl: TemplateRef; items: any; constructor() { this.items = [1, 2]; } + customTrackBy(index: number, item: any): string { return item['id']; } } @Component({selector: 'outer-cmp'}) 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 f21f7a29cc..8962362ab3 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 @@ -14,11 +14,17 @@ import { } from 'angular2/src/core/change_detection/differs/default_iterable_differ'; import {NumberWrapper} from 'angular2/src/facade/lang'; -import {ListWrapper, MapWrapper} from 'angular2/src/facade/collection'; +import {ListWrapper} from 'angular2/src/facade/collection'; import {TestIterable} from '../../../core/change_detection/iterable'; import {iterableChangesAsString} from '../../../core/change_detection/util'; +class ItemWithId { + constructor(private id: string) {} + + toString() { return `{id: ${this.id}}` } +} + // todo(vicb): UnmodifiableListView / frozen object when implemented export function main() { describe('iterable differ', function() { @@ -254,6 +260,7 @@ export function main() { })); }); + it('should support duplicates', () => { let l = ['a', 'a', 'a', 'b', 'b']; differ.check(l); @@ -324,5 +331,80 @@ export function main() { }); }); }); + + describe('trackBy function', function() { + var differ; + + var trackByItemId = (index: number, item: any): any => item.id; + + var buildItemList = + (list: string[]) => { return list.map((val) => {return new ItemWithId(val)}) }; + + beforeEach(() => { differ = new DefaultIterableDiffer(trackByItemId); }); + + it('should not treat maps as new with track by function', () => { + + let l = buildItemList(['a', 'b', 'c']); + differ.check(l); + expect(differ.toString()) + .toEqual(iterableChangesAsString({ + collection: [`{id: a}[null->0]`, `{id: b}[null->1]`, `{id: c}[null->2]`], + additions: [`{id: a}[null->0]`, `{id: b}[null->1]`, `{id: c}[null->2]`] + })); + + l = buildItemList(['a', 'b', 'c']); + differ.check(l); + expect(differ.toString()) + .toEqual(iterableChangesAsString({ + collection: [`{id: a}`, `{id: b}`, `{id: c}`], + previous: [`{id: a}`, `{id: b}`, `{id: c}`] + })); + }); + + it('should track moves normally with track by function', () => { + let l = buildItemList(['a', 'b', 'c']); + differ.check(l); + + l = buildItemList(['b', 'a', 'c']); + differ.check(l); + expect(differ.toString()) + .toEqual(iterableChangesAsString({ + collection: ['{id: b}[1->0]', '{id: a}[0->1]', '{id: c}'], + previous: ['{id: a}[0->1]', '{id: b}[1->0]', '{id: c}'], + moves: ['{id: b}[1->0]', '{id: a}[0->1]'] + })); + + }); + + it('should track duplicate reinsertion normally with track by function', () => { + let l = buildItemList(['a', 'a']); + differ.check(l); + + l = buildItemList(['b', 'a', 'a']); + differ.check(l); + expect(differ.toString()) + .toEqual(iterableChangesAsString({ + collection: ['{id: b}[null->0]', '{id: a}[0->1]', '{id: a}[1->2]'], + previous: ['{id: a}[0->1]', '{id: a}[1->2]'], + moves: ['{id: a}[0->1]', '{id: a}[1->2]'], + additions: ['{id: b}[null->0]'] + })); + + }); + + it('should track removals normally with track by function', () => { + let l = buildItemList(['a', 'b', 'c']); + differ.check(l); + + ListWrapper.removeAt(l, 2); + differ.check(l); + expect(differ.toString()) + .toEqual(iterableChangesAsString({ + collection: ['{id: a}', '{id: b}'], + previous: ['{id: a}', '{id: b}', '{id: c}[2->null]'], + removals: ['{id: c}[2->null]'] + })); + }); + }); }); } diff --git a/modules/angular2/test/public_api_spec.ts b/modules/angular2/test/public_api_spec.ts index cfb8ce9594..38c4bdc5ad 100644 --- a/modules/angular2/test/public_api_spec.ts +++ b/modules/angular2/test/public_api_spec.ts @@ -290,6 +290,7 @@ var NG_COMMON = [ 'NgFor.ngDoCheck()', 'NgFor.ngForOf=', 'NgFor.ngForTemplate=', + 'NgFor.ngForTrackBy=', 'NgForm', 'NgForm.addControl()', 'NgForm.addControlGroup()', diff --git a/tools/public_api_guard/public_api_spec.ts b/tools/public_api_guard/public_api_spec.ts index 61175d0770..bea8d61740 100644 --- a/tools/public_api_guard/public_api_spec.ts +++ b/tools/public_api_guard/public_api_spec.ts @@ -249,7 +249,7 @@ const CORE = [ 'IterableDiffer.diff(object:any):any', 'IterableDiffer.onDestroy():any', 'IterableDifferFactory', - 'IterableDifferFactory.create(cdRef:ChangeDetectorRef):IterableDiffer', + 'IterableDifferFactory.create(cdRef:ChangeDetectorRef, trackByFn:TrackByFn):IterableDiffer', 'IterableDifferFactory.supports(objects:any):boolean', 'IterableDiffers', 'IterableDiffers.constructor(factories:IterableDifferFactory[])', @@ -441,6 +441,7 @@ const CORE = [ 'TestabilityRegistry.getAllTestabilities():Testability[]', 'TestabilityRegistry.getTestability(elem:any):Testability', 'TestabilityRegistry.registerApplication(token:any, testability:Testability):any', + 'TrackByFn', 'TypeDecorator', 'TypeDecorator.Class(obj:ClassDefinition):ConcreteType', 'TypeDecorator.annotations:any[]', @@ -696,6 +697,7 @@ const COMMON = [ 'NgFor.ngDoCheck():any', 'NgFor.ngForOf=(value:any)', 'NgFor.ngForTemplate=(value:TemplateRef)', + 'NgFor.ngForTrackBy=(value:TrackByFn)', 'NgForm', 'NgForm.addControl(dir:NgControl):void', 'NgForm.addControlGroup(dir:NgControlGroup):void',