Skip to content

Commit 930e0a2

Browse files
author
Alexander Vakrilov
authored
Merge pull request NativeScript#334 from NativeScript/list-change-detection
FIX: All events should execute inside zone
2 parents 1d7cd0f + fdb1daf commit 930e0a2

File tree

5 files changed

+79
-29
lines changed

5 files changed

+79
-29
lines changed

‎nativescript-angular/renderer.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {Inject, Injectable, Optional} from '@angular/core/src/di';
1+
import {Inject, Injectable, Optional, NgZone} from '@angular/core';
22
import {
33
Renderer,
44
RootRenderer,
@@ -28,7 +28,9 @@ export class NativeScriptRootRenderer implements RootRenderer {
2828
constructor(
2929
@Optional() @Inject(APP_ROOT_VIEW) private _rootView: View,
3030
@Inject(DEVICE) device: Device,
31-
private _animationDriver: AnimationDriver) {
31+
private _animationDriver: AnimationDriver,
32+
private _zone: NgZone) {
33+
3234
this._viewUtil = new ViewUtil(device);
3335
}
3436

@@ -52,7 +54,7 @@ export class NativeScriptRootRenderer implements RootRenderer {
5254
renderComponent(componentProto: RenderComponentType): Renderer {
5355
let renderer = this._registeredComponents.get(componentProto.id);
5456
if (isBlank(renderer)) {
55-
renderer = new NativeScriptRenderer(this, componentProto, this._animationDriver);
57+
renderer = new NativeScriptRenderer(this, componentProto, this._animationDriver, this._zone);
5658
this._registeredComponents.set(componentProto.id, renderer);
5759
}
5860
return renderer;
@@ -63,16 +65,18 @@ export class NativeScriptRootRenderer implements RootRenderer {
6365
export class NativeScriptRenderer extends Renderer {
6466
private componentProtoId: string;
6567
private hasComponentStyles: boolean;
66-
private rootRenderer: NativeScriptRootRenderer;
6768

6869
private get viewUtil(): ViewUtil {
6970
return this.rootRenderer.viewUtil;
7071
}
7172

72-
constructor(private _rootRenderer: NativeScriptRootRenderer, private componentProto: RenderComponentType, private animationDriver: AnimationDriver) {
73+
constructor(
74+
private rootRenderer: NativeScriptRootRenderer,
75+
private componentProto: RenderComponentType,
76+
private animationDriver: AnimationDriver,
77+
private zone: NgZone) {
78+
7379
super();
74-
this.rootRenderer = _rootRenderer;
75-
let page = this.rootRenderer.page;
7680
let stylesLength = componentProto.styles.length;
7781
this.componentProtoId = componentProto.id;
7882
for (let i = 0; i < stylesLength; i++) {
@@ -93,7 +97,7 @@ export class NativeScriptRenderer extends Renderer {
9397
}
9498

9599
renderComponent(componentProto: RenderComponentType): Renderer {
96-
return this._rootRenderer.renderComponent(componentProto);
100+
return this.rootRenderer.renderComponent(componentProto);
97101
}
98102

99103
selectRootElement(selector: string): NgView {
@@ -211,7 +215,13 @@ export class NativeScriptRenderer extends Renderer {
211215

212216
public listen(renderElement: NgView, eventName: string, callback: Function): Function {
213217
traceLog('NativeScriptRenderer.listen: ' + eventName);
214-
let zonedCallback = (<any>global).Zone.current.wrap(callback);
218+
// Explicitly wrap in zone
219+
let zonedCallback = (...args) => {
220+
this.zone.run(() => {
221+
callback.apply(undefined, args);
222+
});
223+
};
224+
215225
renderElement.on(eventName, zonedCallback);
216226
if (eventName === View.loadedEvent && renderElement.isLoaded) {
217227
const notifyData = { eventName: View.loadedEvent, object: renderElement };

‎tests/app/tests/renderer-tests.ts

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//make sure you import mocha-config before @angular/core
22
import {assert} from "./test-config";
3-
import {Component, ElementRef, Renderer} from "@angular/core";
3+
import {Component, ElementRef, Renderer, NgZone} from "@angular/core";
44
import {ProxyViewContainer} from "ui/proxy-view-container";
55
import {Red} from "color/known-colors";
66
import {dumpView} from "./test-utils";
@@ -109,7 +109,8 @@ describe('Renderer E2E', () => {
109109
before(() => {
110110
return TestApp.create().then((app) => {
111111
testApp = app;
112-
})
112+
113+
});
113114
});
114115

115116
after(() => {
@@ -163,6 +164,44 @@ describe('Renderer E2E', () => {
163164
});
164165
});
165166

167+
it("executes events inside NgZone when listen is called inside NgZone", (done) => {
168+
const eventName = "someEvent";
169+
const view = new StackLayout();
170+
const evetArg = { eventName, object: view };
171+
const callback = (arg) => {
172+
assert.equal(arg, evetArg);
173+
assert.isTrue(NgZone.isInAngularZone(), "Event should be executed inside NgZone");
174+
done();
175+
};
176+
177+
testApp.zone.run(() => {
178+
testApp.renderer.listen(view, eventName, callback);
179+
});
180+
181+
setTimeout(() => {
182+
testApp.zone.runOutsideAngular(() => {
183+
view.notify(evetArg);
184+
});
185+
}, 10);
186+
});
187+
188+
it("executes events inside NgZone when listen is called outside NgZone", (done) => {
189+
const eventName = "someEvent";
190+
const view = new StackLayout();
191+
const evetArg = { eventName, object: view };
192+
const callback = (arg) => {
193+
assert.equal(arg, evetArg);
194+
assert.isTrue(NgZone.isInAngularZone(), "Event should be executed inside NgZone");
195+
done();
196+
};
197+
198+
testApp.zone.runOutsideAngular(() => {
199+
testApp.renderer.listen(view, eventName, callback);
200+
201+
view.notify(evetArg);
202+
});
203+
});
204+
166205
describe("Structural directives", () => {
167206
it("ngIf hides component when false", () => {
168207
return testApp.loadComponent(NgIfLabel).then((componentRef) => {
@@ -180,7 +219,7 @@ describe('Renderer E2E', () => {
180219
testApp.appRef.tick();
181220
assert.equal("(ProxyViewContainer (template), (Label))", dumpView(componentRoot));
182221
});
183-
})
222+
});
184223

185224
it("ngFor creates element for each item", () => {
186225
return testApp.loadComponent(NgForLabel).then((componentRef) => {
@@ -212,8 +251,8 @@ describe('Renderer E2E', () => {
212251
assert.equal("(ProxyViewContainer (template), (Label[text=one]), (Label[text=new]), (Label[text=two]), (Label[text=three]))", dumpView(componentRoot, true));
213252
});
214253
});
215-
})
216-
})
254+
});
255+
});
217256

218257
describe('Renderer createElement', () => {
219258
let testApp: TestApp = null;

‎tests/app/tests/style-properties.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,10 @@ describe("Setting style properties", () => {
1414

1515
beforeEach(() => {
1616
const animationDriver = new NativeScriptAnimationDriver()
17-
const rootRenderer = new NativeScriptRootRenderer(null, device, animationDriver);
17+
const rootRenderer = new NativeScriptRootRenderer(null, device, animationDriver, null);
1818
const componentType = new RenderComponentType("id", "templateUrl", 0,
1919
null, []);
20-
renderer = new NativeScriptRenderer(rootRenderer, componentType, animationDriver);
20+
renderer = new NativeScriptRenderer(rootRenderer, componentType, animationDriver, null);
2121
element = <NgView><any>new TextField();
2222
});
2323

‎tests/app/tests/test-app.ts

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
//make sure you import mocha-config before @angular/core
22
import {bootstrap, ProviderArray} from "nativescript-angular/application";
33
import {Type, Component, ComponentRef, DynamicComponentLoader,
4-
ViewChild, ElementRef, provide, ApplicationRef, Renderer, ViewContainerRef
4+
ViewChild, ElementRef, provide, ApplicationRef, Renderer, ViewContainerRef, NgZone
55
} from "@angular/core";
66

7-
import {View} from "ui/core/view";
87
import {GridLayout} from "ui/layouts/grid-layout";
98
import {LayoutBase} from "ui/layouts/layout-base";
109
import {topmost} from 'ui/frame';
@@ -21,7 +20,8 @@ export class TestApp {
2120
constructor(public loader: DynamicComponentLoader,
2221
public elementRef: ViewContainerRef,
2322
public appRef: ApplicationRef,
24-
public renderer: Renderer) {
23+
public renderer: Renderer,
24+
public zone: NgZone) {
2525
}
2626

2727
public loadComponent(type: Type): Promise<ComponentRef<any>> {
@@ -49,7 +49,7 @@ export class TestApp {
4949
}
5050
}
5151

52-
var runningApps = new Map<any, { hostView: LayoutBase, appRoot: GridLayout, appRef: ApplicationRef }>();
52+
const runningApps = new Map<any, { hostView: LayoutBase, appRoot: GridLayout, appRef: ApplicationRef }>();
5353

5454
export function bootstrapTestApp<T>(appComponentType: new (...args) => T, providers: ProviderArray = []): Promise<T> {
5555
const page = topmost().currentPage;
@@ -61,17 +61,18 @@ export function bootstrapTestApp<T>(appComponentType: new (...args) => T, provid
6161
viewRoot.opacity = 0.7;
6262
GridLayout.setRowSpan(rootLayout, 50);
6363
GridLayout.setColumnSpan(rootLayout, 50);
64-
64+
6565
const rootViewProvider = provide(APP_ROOT_VIEW, { useValue: viewRoot });
6666
return bootstrap(appComponentType, providers.concat(rootViewProvider)).then((componentRef) => {
67-
componentRef.injector.get(ApplicationRef)
67+
componentRef.injector.get(ApplicationRef);
6868
const testApp = componentRef.instance;
69-
70-
runningApps.set(testApp, {
71-
hostView: rootLayout,
72-
appRoot: viewRoot,
73-
appRef: componentRef.injector.get(ApplicationRef) });
74-
69+
70+
runningApps.set(testApp, {
71+
hostView: rootLayout,
72+
appRoot: viewRoot,
73+
appRef: componentRef.injector.get(ApplicationRef)
74+
});
75+
7576
return testApp;
7677
});
7778
}

‎tests/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
"version": "2.1.1"
66
},
77
"tns-ios": {
8-
"version": "2.0.1"
8+
"version": "2.1.1"
99
}
1010
},
1111
"name": "ngtests",

0 commit comments

Comments
 (0)