Skip to content

Commit

Permalink
fix(core): fix the key/value differ
Browse files Browse the repository at this point in the history 
fixes angular#15457
  • Loading branch information
@vicb
vicb committed Mar 27, 2017
1 parent 6605dd1 commit a546068
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 114 deletions.
182 changes: 76 additions & 106 deletions packages/core/src/change_detection/differs/default_keyvalue_differ.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,19 @@ export class DefaultKeyValueDifferFactory<K, V> implements KeyValueDifferFactory
}

export class DefaultKeyValueDiffer<K, V> implements KeyValueDiffer<K, V>, KeyValueChanges<K, V> {
private _records: Map<K, V> = new Map<K, V>();
private _records = new Map<K, KeyValueChangeRecord_<K, V>>();

private _mapHead: KeyValueChangeRecord_<K, V> = null;
private _mapTail: KeyValueChangeRecord_<K, V> = null;

private _previousMapHead: KeyValueChangeRecord_<K, V> = null;

private _changesHead: KeyValueChangeRecord_<K, V> = null;
private _changesTail: KeyValueChangeRecord_<K, V> = null;

private _additionsHead: KeyValueChangeRecord_<K, V> = null;
private _additionsTail: KeyValueChangeRecord_<K, V> = null;

private _removalsHead: KeyValueChangeRecord_<K, V> = null;
private _removalsTail: KeyValueChangeRecord_<K, V> = null;

Expand Down Expand Up @@ -92,65 +98,90 @@ export class DefaultKeyValueDiffer<K, V> implements KeyValueDiffer<K, V>, KeyVal

check(map: Map<any, any>|{[k: string]: any}): boolean {
this._reset();
const records = this._records;
let oldSeqRecord: KeyValueChangeRecord_<K, V> = this._mapHead;
let lastOldSeqRecord: KeyValueChangeRecord_<K, V> = null;
let lastNewSeqRecord: KeyValueChangeRecord_<K, V> = null;
let seqChanged: boolean = false;

// Start by assuming that all elements have been removed and store them in a linked-list
// They use the `record._next` and `record._prev` pointers from the state of the map in the
// previous iteration.
// When any of the previous keys is encountered in the current state of the map, we remove the
// element from the removal list.
this._removalsHead = this._mapHead;
this._removalsTail = this._mapTail;

this._mapHead = null;
this._mapTail = null;

this._forEach(map, (value: any, key: any) => {
let newSeqRecord: any;
if (oldSeqRecord && key === oldSeqRecord.key) {
newSeqRecord = oldSeqRecord;
this._maybeAddToChanges(newSeqRecord, value);
} else {
seqChanged = true;
if (oldSeqRecord !== null) {
this._removeFromSeq(lastOldSeqRecord, oldSeqRecord);
this._addToRemovals(oldSeqRecord);
}
if (records.has(key)) {
newSeqRecord = records.get(key);
this._maybeAddToChanges(newSeqRecord, value);
} else {
newSeqRecord = new KeyValueChangeRecord_<K, V>(key);
records.set(key, newSeqRecord);
newSeqRecord.currentValue = value;
this._addToAdditions(newSeqRecord);
}
}
const previousRecord = this._records.get(key);

if (seqChanged) {
if (this._isInRemovals(newSeqRecord)) {
this._removeFromRemovals(newSeqRecord);
}
if (lastNewSeqRecord == null) {
this._mapHead = newSeqRecord;
} else {
lastNewSeqRecord._next = newSeqRecord;
}
if (previousRecord) {
this._maybeAddToChanges(previousRecord, value);
this._removeFromRemovals(previousRecord);
this._addToSeq(previousRecord);
} else {
const newRecord = new KeyValueChangeRecord_<K, V>(key);
this._records.set(key, newRecord);
newRecord.currentValue = value;
this._addToAdditions(newRecord);
this._addToSeq(newRecord);
}
lastOldSeqRecord = oldSeqRecord;
lastNewSeqRecord = newSeqRecord;
oldSeqRecord = oldSeqRecord && oldSeqRecord._next;
});
this._truncate(lastOldSeqRecord, oldSeqRecord);


for (let record = this._removalsHead; record; record = record._next) {
this._records.delete(record.key);
record._nextRemoved = record._next;
record.previousValue = record.currentValue;
record.currentValue = null;
}

return this.isDirty;
}

// Because we start by assuming that all previous key have been removed, we need to remove
// the keys that are still in the new state of the map to the removal list.
private _removeFromRemovals(record: KeyValueChangeRecord_<K, V>) {
const prev = record._prev;
const next = record._next;
if (prev === null) {
this._removalsHead = next;
} else {
prev._next = next;
}
if (next === null) {
this._removalsTail = prev;
} else {
next._prev = prev;
}
record._next = null;
record._prev = null;
}

private _addToSeq(record: KeyValueChangeRecord_<K, V>) {
if (this._mapHead === null) {
this._mapHead = record;
this._mapTail = record;
} else {
record._prev = this._mapTail;
this._mapTail._next = record;
this._mapTail = record;
}
}

/** @internal */
_reset() {
if (this.isDirty) {
let record: KeyValueChangeRecord_<K, V>;
// Record the state of the mapping
for (record = this._previousMapHead = this._mapHead; record !== null; record = record._next) {
// let `_previousSeqHead` contain the state of the map before the changes
this._previousMapHead = this._mapHead;
for (record = this._previousMapHead; record !== null; record = record._next) {
record._nextPrevious = record._next;
}

// Update `record.previousValue` with the value of the item before the changes
// We need to update all changed items (that's those which have been added and changed)
for (record = this._changesHead; record !== null; record = record._nextChanged) {
record.previousValue = record.currentValue;
}

for (record = this._additionsHead; record != null; record = record._nextAdded) {
record.previousValue = record.currentValue;
}
Expand All @@ -161,27 +192,7 @@ export class DefaultKeyValueDiffer<K, V> implements KeyValueDiffer<K, V>, KeyVal
}
}

private _truncate(lastRecord: KeyValueChangeRecord_<K, V>, record: KeyValueChangeRecord_<K, V>) {
while (record !== null) {
if (lastRecord === null) {
this._mapHead = null;
} else {
lastRecord._next = null;
}
const nextRecord = record._next;
this._addToRemovals(record);
lastRecord = record;
record = nextRecord;
}

for (let rec: KeyValueChangeRecord_<K, V> = this._removalsHead; rec !== null;
rec = rec._nextRemoved) {
rec.previousValue = rec.currentValue;
rec.currentValue = null;
this._records.delete(rec.key);
}
}

// Add the record or a given key to the list of changes only when the value has actually changed
private _maybeAddToChanges(record: KeyValueChangeRecord_<K, V>, newValue: any): void {
if (!looseIdentical(newValue, record.currentValue)) {
record.previousValue = record.currentValue;
Expand All @@ -190,47 +201,6 @@ export class DefaultKeyValueDiffer<K, V> implements KeyValueDiffer<K, V>, KeyVal
}
}

private _isInRemovals(record: KeyValueChangeRecord_<K, V>) {
return record === this._removalsHead || record._nextRemoved !== null ||
record._prevRemoved !== null;
}

private _addToRemovals(record: KeyValueChangeRecord_<K, V>) {
if (this._removalsHead === null) {
this._removalsHead = this._removalsTail = record;
} else {
this._removalsTail._nextRemoved = record;
record._prevRemoved = this._removalsTail;
this._removalsTail = record;
}
}

private _removeFromSeq(prev: KeyValueChangeRecord_<K, V>, record: KeyValueChangeRecord_<K, V>) {
const next = record._next;
if (prev === null) {
this._mapHead = next;
} else {
prev._next = next;
}
record._next = null;
}

private _removeFromRemovals(record: KeyValueChangeRecord_<K, V>) {
const prev = record._prevRemoved;
const next = record._nextRemoved;
if (prev === null) {
this._removalsHead = next;
} else {
prev._nextRemoved = next;
}
if (next === null) {
this._removalsTail = prev;
} else {
next._prevRemoved = prev;
}
record._prevRemoved = record._nextRemoved = null;
}

private _addToAdditions(record: KeyValueChangeRecord_<K, V>) {
if (this._additionsHead === null) {
this._additionsHead = this._additionsTail = record;
Expand Down Expand Up @@ -303,12 +273,12 @@ class KeyValueChangeRecord_<K, V> implements KeyValueChangeRecord<K, V> {
/** @internal */
_next: KeyValueChangeRecord_<K, V> = null;
/** @internal */
_prev: KeyValueChangeRecord_<K, V> = null;
/** @internal */
_nextAdded: KeyValueChangeRecord_<K, V> = null;
/** @internal */
_nextRemoved: KeyValueChangeRecord_<K, V> = null;
/** @internal */
_prevRemoved: KeyValueChangeRecord_<K, V> = null;
/** @internal */
_nextChanged: KeyValueChangeRecord_<K, V> = null;

constructor(public key: K) {}
Expand Down
24 changes: 16 additions & 8 deletions packages/core/test/change_detection/differs/default_keyvalue_differ_spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
*/

import {DefaultKeyValueDiffer, DefaultKeyValueDifferFactory} from '@angular/core/src/change_detection/differs/default_keyvalue_differ';
import {afterEach, beforeEach, describe, expect, it} from '@angular/core/testing/src/testing_internal';
import {kvChangesAsString} from '../../change_detection/util';

// todo(vicb): Update the code & tests for object equality
Expand Down Expand Up @@ -55,21 +54,17 @@ export function main() {
});

it('should expose previous and current value', () => {
let previous: any /** TODO #9100 */, current: any /** TODO #9100 */;

m.set(1, 10);
differ.check(m);

m.set(1, 20);
differ.check(m);

differ.forEachChangedItem((record: any /** TODO #9100 */) => {
previous = record.previousValue;
current = record.currentValue;
differ.forEachChangedItem((record: any) => {
expect(record.previousValue).toEqual(10);
expect(record.currentValue).toEqual(20);
});

expect(previous).toEqual(10);
expect(current).toEqual(20);
});

it('should do basic map watching', () => {
Expand Down Expand Up @@ -198,6 +193,19 @@ export function main() {
changes: ['b[0->1]', 'a[0->1]']
}));
});

it('should when the first item is moved', () => {
differ.check({a: 'a', b: 'b'});
differ.check({c: 'c', a: 'a'});

expect(differ.toString()).toEqual(kvChangesAsString({
map: ['c[null->c]', 'a'],
previous: ['a', 'b[b->null]'],
additions: ['c[null->c]'],
removals: ['b[b->null]']
}));
});

});

describe('diff', () => {
Expand Down

0 comments on commit a546068

Please sign in to comment.