Skip to content

Commit e458624

Browse files
vsavkinmhevery
vsavkin
authored and
mhevery
committed
refactor(change_detection): removed NO_CHANGED and replaced it with WrappedValue
1 parent 4c1e978 commit e458624

11 files changed

Lines changed: 78 additions & 45 deletions

File tree

‎modules/angular2/change_detection.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export {DynamicChangeDetector} from './src/change_detection/dynamic_change_detec
2323
export {ChangeDetectorRef} from './src/change_detection/change_detector_ref';
2424
export {PipeRegistry} from './src/change_detection/pipes/pipe_registry';
2525
export {uninitialized} from './src/change_detection/change_detection_util';
26-
export {NO_CHANGE, Pipe} from './src/change_detection/pipes/pipe';
26+
export {WrappedValue, Pipe} from './src/change_detection/pipes/pipe';
2727
export {
2828
defaultPipes, DynamicChangeDetection, JitChangeDetection, defaultPipeRegistry
2929
} from './src/change_detection/change_detection';

‎modules/angular2/src/change_detection/change_detection_jit_generator.es6‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,8 @@ if (${pipe} === ${UTIL}.unitialized()) {
156156
}
157157

158158
${newValue} = ${pipe}.transform(${context});
159-
if (! ${UTIL}.noChangeMarker(${newValue})) {
159+
if (${oldValue} !== ${newValue}) {
160+
${newValue} = ${UTIL}.unwrapValue(${newValue});
160161
${change} = true;
161162
${update}
162163
${addToChanges}

‎modules/angular2/src/change_detection/change_detection_util.js‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import {isPresent, isBlank, BaseException, Type} from 'angular2/src/facade/lang'
22
import {List, ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
33
import {ProtoRecord} from './proto_record';
44
import {ExpressionChangedAfterItHasBeenChecked} from './exceptions';
5-
import {NO_CHANGE} from './pipes/pipe';
5+
import {WrappedValue} from './pipes/pipe';
66
import {CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH} from './constants';
77

88
export var uninitialized = new Object();
@@ -109,8 +109,12 @@ export class ChangeDetectionUtil {
109109
return obj[args[0]];
110110
}
111111

112-
static noChangeMarker(value):boolean {
113-
return value === NO_CHANGE;
112+
static unwrapValue(value:any):any {
113+
if (value instanceof WrappedValue) {
114+
return value.wrapped;
115+
} else {
116+
return value;
117+
}
114118
}
115119

116120
static throwOnChange(proto:ProtoRecord, change) {

‎modules/angular2/src/change_detection/dynamic_change_detector.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,11 +233,13 @@ export class DynamicChangeDetector extends AbstractChangeDetector {
233233
_pipeCheck(proto:ProtoRecord) {
234234
var context = this._readContext(proto);
235235
var pipe = this._pipeFor(proto, context);
236+
var prevValue = this._readSelf(proto);
236237

237238
var newValue = pipe.transform(context);
238239

239-
if (! ChangeDetectionUtil.noChangeMarker(newValue)) {
240-
var prevValue = this._readSelf(proto);
240+
if (!isSame(prevValue, newValue)) {
241+
newValue = ChangeDetectionUtil.unwrapValue(newValue);
242+
241243
this._writeSelf(proto, newValue);
242244
this._setChanged(proto, true);
243245

‎modules/angular2/src/change_detection/pipes/async_pipe.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {Observable, ObservableWrapper} from 'angular2/src/facade/async';
22
import {isBlank, isPresent} from 'angular2/src/facade/lang';
3-
import {Pipe, NO_CHANGE} from './pipe';
3+
import {Pipe, WrappedValue} from './pipe';
44
import {ChangeDetectorRef} from '../change_detector_ref';
55

66
/**
@@ -67,10 +67,10 @@ export class AsyncPipe extends Pipe {
6767
}
6868

6969
if (this._latestValue === this._latestReturnedValue) {
70-
return NO_CHANGE;
70+
return this._latestReturnedValue;
7171
} else {
7272
this._latestReturnedValue = this._latestValue;
73-
return this._latestValue;
73+
return WrappedValue.wrap(this._latestValue);
7474
}
7575
}
7676

‎modules/angular2/src/change_detection/pipes/iterable_changes.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
looseIdentical,
1515
} from 'angular2/src/facade/lang';
1616

17-
import {NO_CHANGE, Pipe} from './pipe';
17+
import {WrappedValue, Pipe} from './pipe';
1818

1919
export class IterableChangesFactory {
2020
supports(obj):boolean {
@@ -117,9 +117,9 @@ export class IterableChanges extends Pipe {
117117

118118
transform(collection){
119119
if (this.check(collection)) {
120-
return this;
120+
return WrappedValue.wrap(this);
121121
} else {
122-
return NO_CHANGE;
122+
return this;
123123
}
124124
}
125125

‎modules/angular2/src/change_detection/pipes/keyvalue_changes.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {ListWrapper, MapWrapper, StringMapWrapper} from 'angular2/src/facade/collection';
22
import {stringify, looseIdentical, isJsObject} from 'angular2/src/facade/lang';
33

4-
import {NO_CHANGE, Pipe} from './pipe';
4+
import {WrappedValue, Pipe} from './pipe';
55

66
/**
77
* @exportedAs angular2/pipes
@@ -54,9 +54,9 @@ export class KeyValueChanges extends Pipe {
5454

5555
transform(map){
5656
if (this.check(map)) {
57-
return this;
57+
return WrappedValue.wrap(this);
5858
} else {
59-
return NO_CHANGE;
59+
return this;
6060
}
6161
}
6262

‎modules/angular2/src/change_detection/pipes/null_pipe.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {isBlank} from 'angular2/src/facade/lang';
2-
import {Pipe, NO_CHANGE} from './pipe';
2+
import {Pipe, WrappedValue} from './pipe';
33

44
/**
55
* @exportedAs angular2/pipes
@@ -35,9 +35,9 @@ export class NullPipe extends Pipe {
3535
transform(value) {
3636
if (! this.called) {
3737
this.called = true;
38-
return null;
38+
return WrappedValue.wrap(null);
3939
} else {
40-
return NO_CHANGE;
40+
return null;
4141
}
4242
}
4343
}

‎modules/angular2/src/change_detection/pipes/pipe.js‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,33 @@
11
/**
2-
* Indicates that the result of a {@link Pipe} transformation has not changed since the last time the pipe was called.
2+
* Indicates that the result of a {@link Pipe} transformation has changed even though the reference has not changed.
33
*
4-
* Suppose we have a pipe that computes changes in an array by performing a simple diff operation. If
5-
* we call this pipe with the same array twice, it will return `NO_CHANGE` the second time.
4+
* The wrapped value will be unwrapped by change detection, and the unwrapped value will be stored.
65
*
76
* @exportedAs angular2/pipes
87
*/
8+
export class WrappedValue {
9+
wrapped:any;
910

10-
export var NO_CHANGE = new Object();
11+
constructor(wrapped:any) {
12+
this.wrapped = wrapped;
13+
}
14+
15+
static wrap(value:any):WrappedValue {
16+
var w = _wrappedValues[_wrappedIndex++ % 5];
17+
w.wrapped = value;
18+
return w;
19+
}
20+
}
21+
22+
var _wrappedValues = [
23+
new WrappedValue(null),
24+
new WrappedValue(null),
25+
new WrappedValue(null),
26+
new WrappedValue(null),
27+
new WrappedValue(null)
28+
];
29+
30+
var _wrappedIndex = 0;
1131

1232
/**
1333
* An interface for extending the list of pipes known to Angular.

‎modules/angular2/test/change_detection/change_detection_spec.js‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {Lexer} from 'angular2/src/change_detection/parser/lexer';
88
import {Locals} from 'angular2/src/change_detection/parser/locals';
99

1010
import {ChangeDispatcher, DynamicChangeDetector, ChangeDetectionError, BindingRecord, DirectiveRecord, DirectiveIndex,
11-
PipeRegistry, Pipe, NO_CHANGE, CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH, DEFAULT} from 'angular2/change_detection';
11+
PipeRegistry, Pipe, CHECK_ALWAYS, CHECK_ONCE, CHECKED, DETACHED, ON_PUSH, DEFAULT, WrappedValue} from 'angular2/change_detection';
1212

1313
import {JitProtoChangeDetector, DynamicProtoChangeDetector} from 'angular2/src/change_detection/proto_change_detector';
1414

@@ -733,24 +733,35 @@ export function main() {
733733
});
734734
});
735735

736-
it("should do nothing when returns NO_CHANGE", () => {
736+
it("should do nothing when no change", () => {
737737
var registry = new FakePipeRegistry('pipe', () => new IdentityPipe())
738738
var ctx = new Person("Megatron");
739739

740740
var c = createChangeDetector("memo", "name | pipe", ctx, null, registry);
741741
var cd = c["changeDetector"];
742742
var dispatcher = c["dispatcher"];
743743

744-
cd.detectChanges();
745744
cd.detectChanges();
746745

747746
expect(dispatcher.log).toEqual(['memo=Megatron']);
748747

749-
ctx.name = "Optimus Prime";
750748
dispatcher.clear();
751749
cd.detectChanges();
752750

753-
expect(dispatcher.log).toEqual(['memo=Optimus Prime']);
751+
expect(dispatcher.log).toEqual([]);
752+
});
753+
754+
it("should unwrap the wrapped value", () => {
755+
var registry = new FakePipeRegistry('pipe', () => new WrappedPipe())
756+
var ctx = new Person("Megatron");
757+
758+
var c = createChangeDetector("memo", "name | pipe", ctx, null, registry);
759+
var cd = c["changeDetector"];
760+
var dispatcher = c["dispatcher"];
761+
762+
cd.detectChanges();
763+
764+
expect(dispatcher.log).toEqual(['memo=Megatron']);
754765
});
755766
});
756767
});
@@ -798,19 +809,14 @@ class OncePipe extends Pipe {
798809
}
799810

800811
class IdentityPipe extends Pipe {
801-
state:any;
802-
803-
supports(newValue) {
804-
return true;
812+
transform(value) {
813+
return value;
805814
}
815+
}
806816

817+
class WrappedPipe extends Pipe {
807818
transform(value) {
808-
if (this.state === value) {
809-
return NO_CHANGE;
810-
} else {
811-
this.state = value;
812-
return value;
813-
}
819+
return WrappedValue.wrap(value);
814820
}
815821
}
816822

0 commit comments

Comments
 (0)