honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(ngFor): update view locals if identity changes 

Closes #6923
This commit is contained in:
Kara Erickson 2016-02-05 16:32:24 -08:00 committed by Kara
parent 6f1ef33e32
commit 0f10624b08
6 changed files with 146 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
modules/angular2/src/common/directives/ng_for.ts
View File

@ -117,6 +117,11 @@ export class NgFor implements DoCheck {
var viewRef = <EmbeddedViewRef>this._viewContainer.get(i);
viewRef.setLocal('last', i === ilen - 1);
}
changes.forEachIdentityChange((record) => {
var viewRef = <EmbeddedViewRef>this._viewContainer.get(record.currentIndex);
viewRef.setLocal('\$implicit', record.item);
});
}
private _perViewChange(view, record) {

57
modules/angular2/src/core/change_detection/differs/default_iterable_differ.ts
View File

@ -40,6 +40,9 @@ export class DefaultIterableDiffer implements IterableDiffer {
private _movesTail: CollectionChangeRecord = null;
private _removalsHead: CollectionChangeRecord = null;
private _removalsTail: CollectionChangeRecord = null;
// Keeps track of records where custom track by is the same, but item identity has changed
private _identityChangesHead: CollectionChangeRecord = null;
private _identityChangesTail: CollectionChangeRecord = null;
constructor(private _trackByFn?: TrackByFn) {
this._trackByFn = isPresent(this._trackByFn) ? this._trackByFn : trackByIdentity;
@ -84,6 +87,13 @@ export class DefaultIterableDiffer implements IterableDiffer {
}
}
forEachIdentityChange(fn: Function) {
var record: CollectionChangeRecord;
for (record = this._identityChangesHead; record !== null; record = record._nextIdentityChange) {
fn(record);
}
}
diff(collection: any): DefaultIterableDiffer {
if (isBlank(collection)) collection = [];
if (!isListLikeIterable(collection)) {
@ -123,7 +133,7 @@ export class DefaultIterableDiffer implements IterableDiffer {
// TODO(misko): can we limit this to duplicates only?
record = this._verifyReinsertion(record, item, itemTrackBy, index);
}
record.item = item;
if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item);
}
record = record._next;
@ -135,9 +145,12 @@ export class DefaultIterableDiffer implements IterableDiffer {
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);
} 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);
}
record = record._next;
index++;
@ -150,9 +163,12 @@ export class DefaultIterableDiffer implements IterableDiffer {
return this.isDirty;
}
// CollectionChanges is considered dirty if it has any additions, moves or removals.
/* CollectionChanges is considered dirty if it has any additions, moves, removals, or identity
* changes.
*/
get isDirty(): boolean {
return this._additionsHead !== null || this._movesHead !== null || this._removalsHead !== null;
return this._additionsHead !== null || this._movesHead !== null ||
this._removalsHead !== null || this._identityChangesHead !== null;
}
/**
@ -183,6 +199,7 @@ export class DefaultIterableDiffer implements IterableDiffer {
}
this._movesHead = this._movesTail = null;
this._removalsHead = this._removalsTail = null;
this._identityChangesHead = this._identityChangesTail = null;
// todo(vicb) when assert gets supported
// assert(!this.isDirty);
@ -216,12 +233,18 @@ export class DefaultIterableDiffer implements IterableDiffer {
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.
// But first we need to check if identity changed, so we can update in view if necessary
if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item);
this._moveAfter(record, previousRecord, index);
} else {
// Never seen it, check evicted list.
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.
// But first we need to check if identity changed, so we can update in view if necessary
if (!looseIdentical(record.item, item)) this._addIdentityChange(record, item);
this._reinsertAfter(record, previousRecord, index);
} else {
// It is a new item: add it.
@ -269,7 +292,6 @@ export class DefaultIterableDiffer implements IterableDiffer {
record.currentIndex = index;
this._addToMoves(record, index);
}
record.item = item;
return record;
}
@ -469,6 +491,18 @@ export class DefaultIterableDiffer implements IterableDiffer {
return record;
}
/** @internal */
_addIdentityChange(record: CollectionChangeRecord, item: any) {
record.item = item;
if (this._identityChangesTail === null) {
this._identityChangesTail = this._identityChangesHead = record;
} else {
this._identityChangesTail = this._identityChangesTail._nextIdentityChange = record;
}
return record;
}
toString(): string {
var list = [];
this.forEachItem((record) => list.push(record));
@ -485,9 +519,13 @@ export class DefaultIterableDiffer implements IterableDiffer {
var removals = [];
this.forEachRemovedItem((record) => removals.push(record));
var identityChanges = [];
this.forEachIdentityChange((record) => identityChanges.push(record));
return "collection: " + list.join(', ') + "\n" + "previous: " + previous.join(', ') + "\n" +
"additions: " + additions.join(', ') + "\n" + "moves: " + moves.join(', ') + "\n" +
"removals: " + removals.join(', ') + "\n";
"removals: " + removals.join(', ') + "\n" + "identityChanges: " +
identityChanges.join(', ') + "\n";
}
}
@ -513,6 +551,9 @@ export class CollectionChangeRecord {
_nextAdded: CollectionChangeRecord = null;
/** @internal */
_nextMoved: CollectionChangeRecord = null;
/** @internal */
_nextIdentityChange: CollectionChangeRecord = null;
constructor(public item: any, public trackById: any) {}

76
modules/angular2/test/common/directives/ng_for_spec.ts
View File

@ -361,30 +361,64 @@ export function main() {
});
}));
it('should use custom track by if function is provided',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template =
`<template ngFor #item [ngForOf]="items" [ngForTrackBy]="customTrackBy" #i="index">
describe('track by', function() {
it('should not replace tracked items',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template =
`<template ngFor #item [ngForOf]="items" [ngForTrackBy]="customTrackBy" #i="index">
<p>{{items[i]}}</p>
</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();
});
}));
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();
});
}));
it('should update implicit local variable on view',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template =
`<div><template ngFor #item [ngForOf]="items" [ngForTrackBy]="customTrackBy">{{item['color']}}</template></div>`;
tcb.overrideTemplate(TestComponent, template)
.createAsync(TestComponent)
.then((fixture) => {
fixture.debugElement.componentInstance.items = [{'id': 'a', 'color': 'blue'}];
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('blue');
fixture.debugElement.componentInstance.items = [{'id': 'a', 'color': 'red'}];
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('red');
async.done();
});
}));
it('should move items around and keep them updated ',
inject([TestComponentBuilder, AsyncTestCompleter], (tcb: TestComponentBuilder, async) => {
var template =
`<div><template ngFor #item [ngForOf]="items" [ngForTrackBy]="customTrackBy">{{item['color']}}</template></div>`;
tcb.overrideTemplate(TestComponent, template)
.createAsync(TestComponent)
.then((fixture) => {
fixture.debugElement.componentInstance.items =
[{'id': 'a', 'color': 'blue'}, {'id': 'b', 'color': 'yellow'}];
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('blueyellow');
fixture.debugElement.componentInstance.items =
[{'id': 'b', 'color': 'orange'}, {'id': 'a', 'color': 'red'}];
fixture.detectChanges();
expect(fixture.debugElement.nativeElement).toHaveText('orangered');
async.done();
});
}));
});
});
}

35
modules/angular2/test/core/change_detection/differs/default_iterable_differ_spec.ts
View File

@ -25,6 +25,12 @@ class ItemWithId {
toString() { return `{id: ${this.id}}` }
}
class ComplexItem {
constructor(private id: string, private color: string) {}
toString() { return `{id: ${this.id}, color: ${this.color}}` }
}
// todo(vicb): UnmodifiableListView / frozen object when implemented
export function main() {
describe('iterable differ', function() {
@ -342,8 +348,12 @@ export function main() {
beforeEach(() => { differ = new DefaultIterableDiffer(trackByItemId); });
it('should not treat maps as new with track by function', () => {
it('should treat the collection as dirty if identity changes', () => {
differ.diff(buildItemList(['a']));
expect(differ.diff(buildItemList(['a']))).toBe(differ);
});
it('should treat seen records as identity changes, not additions', () => {
let l = buildItemList(['a', 'b', 'c']);
differ.check(l);
expect(differ.toString())
@ -357,11 +367,26 @@ export function main() {
expect(differ.toString())
.toEqual(iterableChangesAsString({
collection: [`{id: a}`, `{id: b}`, `{id: c}`],
identityChanges: [`{id: a}`, `{id: b}`, `{id: c}`],
previous: [`{id: a}`, `{id: b}`, `{id: c}`]
}));
});
it('should track moves normally with track by function', () => {
it('should have updated properties in identity change collection', () => {
let l = [new ComplexItem('a', 'blue'), new ComplexItem('b', 'yellow')];
differ.check(l);
l = [new ComplexItem('a', 'orange'), new ComplexItem('b', 'red')];
differ.check(l);
expect(differ.toString())
.toEqual(iterableChangesAsString({
collection: [`{id: a, color: orange}`, `{id: b, color: red}`],
identityChanges: [`{id: a, color: orange}`, `{id: b, color: red}`],
previous: [`{id: a, color: orange}`, `{id: b, color: red}`]
}));
});
it('should track moves normally', () => {
let l = buildItemList(['a', 'b', 'c']);
differ.check(l);
@ -370,13 +395,14 @@ export function main() {
expect(differ.toString())
.toEqual(iterableChangesAsString({
collection: ['{id: b}[1->0]', '{id: a}[0->1]', '{id: c}'],
identityChanges: ['{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', () => {
it('should track duplicate reinsertion normally', () => {
let l = buildItemList(['a', 'a']);
differ.check(l);
@ -385,6 +411,7 @@ export function main() {
expect(differ.toString())
.toEqual(iterableChangesAsString({
collection: ['{id: b}[null->0]', '{id: a}[0->1]', '{id: a}[1->2]'],
identityChanges: ['{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]']
@ -392,7 +419,7 @@ export function main() {
});
it('should track removals normally with track by function', () => {
it('should track removals normally', () => {
let l = buildItemList(['a', 'b', 'c']);
differ.check(l);

9
modules/angular2/test/core/change_detection/util.ts
View File

@ -1,11 +1,12 @@
import {isBlank, CONST_EXPR} from 'angular2/src/facade/lang';
export function iterableChangesAsString({collection = CONST_EXPR([]), previous = CONST_EXPR([]),
additions = CONST_EXPR([]), moves = CONST_EXPR([]),
removals = CONST_EXPR([])}) {
export function iterableChangesAsString(
{collection = CONST_EXPR([]), previous = CONST_EXPR([]), additions = CONST_EXPR([]),
moves = CONST_EXPR([]), removals = CONST_EXPR([]), identityChanges = CONST_EXPR([])}) {
return "collection: " + collection.join(', ') + "\n" + "previous: " + previous.join(', ') + "\n" +
"additions: " + additions.join(', ') + "\n" + "moves: " + moves.join(', ') + "\n" +
"removals: " + removals.join(', ') + "\n";
"removals: " + removals.join(', ') + "\n" + "identityChanges: " +
identityChanges.join(', ') + "\n";
}
export function kvChangesAsString(

1
modules/angular2/test/public_api_spec.ts
View File

@ -419,6 +419,7 @@ var NG_COMMON = [
'ObservableListDiff.forEachMovedItem():dart',
'ObservableListDiff.forEachPreviousItem():dart',
'ObservableListDiff.forEachRemovedItem():dart',
'ObservableListDiff.forEachIdentityChange():dart',
'ObservableListDiff.isDirty:dart',
'ObservableListDiff.length:dart',
'ObservableListDiff.onDestroy():dart',