Skip to content

Commit aa7dba2

Browse files
committed
feat(ivy): support checkNoChanges (angular#22710)
PR Close angular#22710
1 parent 0bf6fa5 commit aa7dba2

File tree

4 files changed

+227
-14
lines changed

4 files changed

+227
-14
lines changed

packages/core/src/render3/instructions.ts

Lines changed: 60 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,13 @@ let bindingIndex: number;
151151
*/
152152
let cleanup: any[]|null;
153153

154+
/**
155+
* In this mode, any changes in bindings will throw an ExpressionChangedAfterChecked error.
156+
*
157+
* Necessary to support ChangeDetectorRef.checkNoChanges().
158+
*/
159+
let checkNoChangesMode = false;
160+
154161
const enum BindingDirection {
155162
Input,
156163
Output,
@@ -194,9 +201,11 @@ export function enterView(newView: LView, host: LElementNode | LViewNode | null)
194201
* the direction of traversal (up or down the view tree) a bit clearer.
195202
*/
196203
export function leaveView(newView: LView): void {
197-
executeHooks(
198-
currentView.data, currentView.tView.viewHooks, currentView.tView.viewCheckHooks,
199-
creationMode);
204+
if (!checkNoChangesMode) {
205+
executeHooks(
206+
currentView.data, currentView.tView.viewHooks, currentView.tView.viewCheckHooks,
207+
creationMode);
208+
}
200209
// Views should be clean and in update mode after being checked, so these bits are cleared
201210
currentView.flags &= ~(LViewFlags.CreationMode | LViewFlags.Dirty);
202211
currentView.lifecycleStage = LifecycleStage.INIT;
@@ -1135,9 +1144,11 @@ export function containerRefreshStart(index: number): void {
11351144
(previousOrParentNode as LContainerNode).native, undefined,
11361145
`the container's native element should not have been set yet.`);
11371146

1138-
// We need to execute init hooks here so ngOnInit hooks are called in top level views
1139-
// before they are called in embedded views (for backwards compatibility).
1140-
executeInitHooks(currentView, currentView.tView, creationMode);
1147+
if (!checkNoChangesMode) {
1148+
// We need to execute init hooks here so ngOnInit hooks are called in top level views
1149+
// before they are called in embedded views (for backwards compatibility).
1150+
executeInitHooks(currentView, currentView.tView, creationMode);
1151+
}
11411152
}
11421153

11431154
/**
@@ -1270,8 +1281,10 @@ export function embeddedViewEnd(): void {
12701281
* @param elementIndex
12711282
*/
12721283
export function directiveRefresh<T>(directiveIndex: number, elementIndex: number): void {
1273-
executeInitHooks(currentView, currentView.tView, creationMode);
1274-
executeContentHooks(currentView, currentView.tView, creationMode);
1284+
if (!checkNoChangesMode) {
1285+
executeInitHooks(currentView, currentView.tView, creationMode);
1286+
executeContentHooks(currentView, currentView.tView, creationMode);
1287+
}
12751288
const template = (tData[directiveIndex] as ComponentDef<T>).template;
12761289
if (template != null) {
12771290
ngDevMode && assertDataInRange(elementIndex);
@@ -1594,6 +1607,37 @@ export function detectChanges<T>(component: T): void {
15941607
}
15951608

15961609

1610+
/**
1611+
* Checks the change detector and its children, and throws if any changes are detected.
1612+
*
1613+
* This is used in development mode to verify that running change detection doesn't
1614+
* introduce other changes.
1615+
*/
1616+
export function checkNoChanges<T>(component: T): void {
1617+
checkNoChangesMode = true;
1618+
try {
1619+
detectChanges(component);
1620+
} finally {
1621+
checkNoChangesMode = false;
1622+
}
1623+
}
1624+
1625+
/** Throws an ExpressionChangedAfterChecked error if checkNoChanges mode is on. */
1626+
function throwErrorIfNoChangesMode(oldValue: any, currValue: any): never|void {
1627+
if (checkNoChangesMode) {
1628+
let msg =
1629+
`ExpressionChangedAfterItHasBeenCheckedError: Expression has changed after it was checked. Previous value: '${oldValue}'. Current value: '${currValue}'.`;
1630+
if (creationMode) {
1631+
msg +=
1632+
` It seems like the view has been created after its parent and its children have been dirty checked.` +
1633+
` Has it been created in a change detection hook ?`;
1634+
}
1635+
// TODO: include debug context
1636+
throw new Error(msg);
1637+
}
1638+
}
1639+
1640+
15971641
/** Checks the view of the component provided. Does not gate on dirty checks or execute doCheck. */
15981642
function detectChangesInternal<T>(hostView: LView, hostNode: LElementNode, component: T) {
15991643
const componentIndex = hostNode.flags >> LNodeFlags.INDX_SHIFT;
@@ -1672,6 +1716,7 @@ export function bind<T>(value: T | NO_CHANGE): T|NO_CHANGE {
16721716

16731717
const changed: boolean = value !== NO_CHANGE && isDifferent(data[bindingIndex], value);
16741718
if (changed) {
1719+
throwErrorIfNoChangesMode(data[bindingIndex], value);
16751720
data[bindingIndex] = value;
16761721
}
16771722
bindingIndex++;
@@ -1841,14 +1886,17 @@ export function consumeBinding(): any {
18411886
export function bindingUpdated(value: any): boolean {
18421887
ngDevMode && assertNotEqual(value, NO_CHANGE, 'Incoming value should never be NO_CHANGE.');
18431888

1844-
if (creationMode || isDifferent(data[bindingIndex], value)) {
1845-
creationMode && initBindings();
1846-
data[bindingIndex++] = value;
1847-
return true;
1889+
if (creationMode) {
1890+
initBindings();
1891+
} else if (isDifferent(data[bindingIndex], value)) {
1892+
throwErrorIfNoChangesMode(data[bindingIndex], value);
18481893
} else {
18491894
bindingIndex++;
18501895
return false;
18511896
}
1897+
1898+
data[bindingIndex++] = value;
1899+
return true;
18521900
}
18531901

18541902
/** Updates binding if changed, then returns the latest value. */

packages/core/src/render3/view_ref.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef} from '../linker/view_ref';
1010

11-
import {detectChanges, markViewDirty} from './instructions';
11+
import {checkNoChanges, detectChanges, markViewDirty} from './instructions';
1212
import {ComponentTemplate} from './interfaces/definition';
1313
import {LViewNode} from './interfaces/node';
1414
import {LView, LViewFlags} from './interfaces/view';
@@ -195,7 +195,13 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T> {
195195
*/
196196
detectChanges(): void { detectChanges(this.context); }
197197

198-
checkNoChanges(): void { notImplemented(); }
198+
/**
199+
* Checks the change detector and its children, and throws if any changes are detected.
200+
*
201+
* This is used in development mode to verify that running change detection doesn't
202+
* introduce other changes.
203+
*/
204+
checkNoChanges(): void { checkNoChanges(this.context); }
199205
}
200206

201207

packages/core/test/bundling/hello_world/bundle.golden_symbols.json

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@
3535
{
3636
"name": "canInsertNativeNode"
3737
},
38+
{
39+
"name": "checkNoChangesMode"
40+
},
3841
{
3942
"name": "createLNode"
4043
},

packages/core/test/render3/change_detection_spec.ts

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,6 +911,162 @@ describe('change detection', () => {
911911
// TODO(kara): add test for dynamic views once bug fix is in
912912
});
913913

914+
describe('checkNoChanges', () => {
915+
let comp: NoChangesComp;
916+
917+
class NoChangesComp {
918+
value = 1;
919+
doCheckCount = 0;
920+
contentCheckCount = 0;
921+
viewCheckCount = 0;
922+
923+
ngDoCheck() { this.doCheckCount++; }
924+
925+
ngAfterContentChecked() { this.contentCheckCount++; }
926+
927+
ngAfterViewChecked() { this.viewCheckCount++; }
928+
929+
constructor(public cdr: ChangeDetectorRef) {}
930+
931+
static ngComponentDef = defineComponent({
932+
type: NoChangesComp,
933+
tag: 'no-changes-comp',
934+
factory: () => comp = new NoChangesComp(injectChangeDetectorRef()),
935+
template: (ctx: NoChangesComp, cm: boolean) => {
936+
if (cm) {
937+
text(0);
938+
}
939+
textBinding(0, bind(ctx.value));
940+
}
941+
});
942+
}
943+
944+
class AppComp {
945+
value = 1;
946+
947+
constructor(public cdr: ChangeDetectorRef) {}
948+
949+
static ngComponentDef = defineComponent({
950+
type: AppComp,
951+
tag: 'app-comp',
952+
factory: () => new AppComp(injectChangeDetectorRef()),
953+
/**
954+
* {{ value }} -
955+
* <no-changes-comp></no-changes-comp>
956+
*/
957+
template: (ctx: AppComp, cm: boolean) => {
958+
if (cm) {
959+
text(0);
960+
elementStart(1, NoChangesComp);
961+
elementEnd();
962+
}
963+
textBinding(0, interpolation1('', ctx.value, ' - '));
964+
NoChangesComp.ngComponentDef.h(2, 1);
965+
directiveRefresh(2, 1);
966+
}
967+
});
968+
}
969+
970+
it('should throw if bindings in current view have changed', () => {
971+
const comp = renderComponent(NoChangesComp);
972+
973+
expect(() => comp.cdr.checkNoChanges()).not.toThrow();
974+
975+
comp.value = 2;
976+
expect(() => comp.cdr.checkNoChanges())
977+
.toThrowError(
978+
/ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi);
979+
});
980+
981+
it('should throw if interpolations in current view have changed', () => {
982+
const app = renderComponent(AppComp);
983+
984+
expect(() => app.cdr.checkNoChanges()).not.toThrow();
985+
986+
app.value = 2;
987+
expect(() => app.cdr.checkNoChanges())
988+
.toThrowError(
989+
/ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi);
990+
});
991+
992+
it('should throw if bindings in children of current view have changed', () => {
993+
const app = renderComponent(AppComp);
994+
995+
expect(() => app.cdr.checkNoChanges()).not.toThrow();
996+
997+
comp.value = 2;
998+
expect(() => app.cdr.checkNoChanges())
999+
.toThrowError(
1000+
/ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi);
1001+
});
1002+
1003+
it('should throw if bindings in embedded view have changed', () => {
1004+
class EmbeddedViewApp {
1005+
value = 1;
1006+
showing = true;
1007+
1008+
constructor(public cdr: ChangeDetectorRef) {}
1009+
1010+
static ngComponentDef = defineComponent({
1011+
type: EmbeddedViewApp,
1012+
tag: 'embedded-view-app',
1013+
factory: () => new EmbeddedViewApp(injectChangeDetectorRef()),
1014+
/**
1015+
* % if (showing) {
1016+
* {{ value }}
1017+
* %}
1018+
*/
1019+
template: (ctx: EmbeddedViewApp, cm: boolean) => {
1020+
if (cm) {
1021+
container(0);
1022+
}
1023+
containerRefreshStart(0);
1024+
{
1025+
if (ctx.showing) {
1026+
if (embeddedViewStart(0)) {
1027+
text(0);
1028+
}
1029+
textBinding(0, bind(ctx.value));
1030+
embeddedViewEnd();
1031+
}
1032+
}
1033+
containerRefreshEnd();
1034+
}
1035+
});
1036+
}
1037+
1038+
const app = renderComponent(EmbeddedViewApp);
1039+
1040+
expect(() => app.cdr.checkNoChanges()).not.toThrow();
1041+
1042+
app.value = 2;
1043+
expect(() => app.cdr.checkNoChanges())
1044+
.toThrowError(
1045+
/ExpressionChangedAfterItHasBeenCheckedError: .+ Previous value: '1'. Current value: '2'/gi);
1046+
});
1047+
1048+
it('should NOT call lifecycle hooks', () => {
1049+
const app = renderComponent(AppComp);
1050+
expect(comp.doCheckCount).toEqual(1);
1051+
expect(comp.contentCheckCount).toEqual(1);
1052+
expect(comp.viewCheckCount).toEqual(1);
1053+
1054+
comp.value = 2;
1055+
expect(() => app.cdr.checkNoChanges()).toThrow();
1056+
expect(comp.doCheckCount).toEqual(1);
1057+
expect(comp.contentCheckCount).toEqual(1);
1058+
expect(comp.viewCheckCount).toEqual(1);
1059+
});
1060+
1061+
it('should NOT throw if bindings in ancestors of current view have changed', () => {
1062+
const app = renderComponent(AppComp);
1063+
1064+
app.value = 2;
1065+
expect(() => comp.cdr.checkNoChanges()).not.toThrow();
1066+
});
1067+
1068+
});
1069+
9141070
});
9151071

9161072
});

0 commit comments

Comments
 (0)