Skip to content

Commit

Permalink
fix(upgrade): Run digest cycle for upgraded components in a child zone
Browse files Browse the repository at this point in the history
This fixes angular#6385 by running `$rootScope.$digest` in a child zone. This
prevents tasks started in the digest cycle from triggering the digest
again, thus preventing the infinite loop.
  • Loading branch information
heathkit committed Apr 4, 2017
1 parent 25132bf commit f01263b
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 35 deletions.
19 changes: 17 additions & 2 deletions packages/upgrade/src/static/upgrade_module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,23 @@ export class UpgradeModule {
// stabilizing
setTimeout(() => {
const $rootScope = $injector.get('$rootScope');
const subscription =
this.ngZone.onMicrotaskEmpty.subscribe(() => $rootScope.$digest());
const subscription = this.ngZone.onMicrotaskEmpty.subscribe(() => {
if (!Zone.current.get('isNgUpgradeZone')) {
Zone.current
.fork({
name: 'ngUpgrade',
properties: {'isNgUpgradeZone': true},
onInvoke: function(
delegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
callback: Function, applyThis: any, applyArgs: any,
source: string) {
delegate.invoke(targetZone, callback, applyThis, applyArgs, source);
$rootScope.$digest();
},
})
.run(() => {$rootScope.$digest()})
}
});
$rootScope.$on('$destroy', () => { subscription.unsubscribe(); });
}, 0);
}
Expand Down
95 changes: 62 additions & 33 deletions packages/upgrade/test/static/integration/change_detection_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {async} from '@angular/core/testing';
import {BrowserModule} from '@angular/platform-browser';
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
import * as angular from '@angular/upgrade/src/common/angular1';
import {UpgradeComponent, UpgradeModule, downgradeComponent} from '@angular/upgrade/static';
import {UpgradeComponent, UpgradeModule, downgradeComponent, downgradeInjectable} from '@angular/upgrade/static';

import {bootstrap, html} from '../test_helpers';

Expand Down Expand Up @@ -102,8 +102,7 @@ export function main() {

this.zone.onMicrotaskEmpty.subscribe(
() => { expect(element.textContent).toEqual('5'); });

Promise.resolve().then(() => this.valueFromPromise = changes['value'].currentValue);
Promise.resolve().then(() => {this.valueFromPromise = changes['value'].currentValue});
}
}

Expand All @@ -127,35 +126,65 @@ export function main() {
});
}));

// This test demonstrates https://github.com/angular/angular/issues/6385
// which was invalidly fixed by https://github.com/angular/angular/pull/6386
// it('should not trigger $digest from an async operation in a watcher', async(() => {
// @Component({selector: 'my-app', template: ''})
// class AppComponent {
// }

// @NgModule({declarations: [AppComponent], imports: [BrowserModule]})
// class Ng2Module {
// }

// const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
// const ng1Module = angular.module('ng1', []).directive(
// 'myApp', adapter.downgradeNg2Component(AppComponent));

// const element = html('<my-app></my-app>');

// adapter.bootstrap(element, ['ng1']).ready((ref) => {
// let doTimeout = false;
// let timeoutId: number;
// ref.ng1RootScope.$watch(() => {
// if (doTimeout && !timeoutId) {
// timeoutId = window.setTimeout(function() {
// timeoutId = null;
// }, 10);
// }
// });
// doTimeout = true;
// });
// }));
it('should not trigger $digest from an async operation in a watcher', async(() => {
@Component({selector: 'my-app', template: ''})
class AppComponent {
}

@NgModule({
declarations: [AppComponent],
entryComponents: [AppComponent],
imports: [BrowserModule, UpgradeModule]
})
class Ng2Module {
ngDoBootstrap() {}
}

const ng1Module = angular.module('ng1', []).directive(
'myApp', downgradeComponent({component: AppComponent}));

const element = html('<my-app></my-app>');

bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then((upgrade) => {
let doTimeout = false;
let timeoutId: number;
let rootScope = upgrade.$injector.get('$rootScope');
rootScope.$watch(() => {
if (doTimeout && !timeoutId) {
timeoutId = window.setTimeout(function() { timeoutId = null; }, 10);
}
});
doTimeout = true;
});
}));

it('should run async tasks from downgraded services in the angular zone', async(() => {
class Ng2Service {
doAsyncThing() {
window.setTimeout(() => { expect(NgZone.isInAngularZone()).toBe(true); }, 10)
}
}

@NgModule({
imports: [BrowserModule, UpgradeModule],
providers: [
{provide: Ng2Service, useClass: Ng2Service},
]
})
class Ng2Module {
ngDoBootstrap() {}
}

// create the ng1 module that will import an ng2 service
const ng1Module =
angular.module('ng1Module', []).factory('ng2Service', downgradeInjectable(Ng2Service));

bootstrap(platformBrowserDynamic(), Ng2Module, html('<div>'), ng1Module)
.then((upgrade) => {
const ng2Service = upgrade.$injector.get('ng2Service');
const rootScope = upgrade.$injector.get('$rootScope');
rootScope.$watch(() => { ng2Service.doAsyncThing(); });
});
}));
});
}

0 comments on commit f01263b

Please sign in to comment.