From 326b464d20ab581938286e2f177fbb78ea8df89b Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Tue, 20 Nov 2018 19:30:15 +0200 Subject: [PATCH] fix(upgrade): correctly handle nested downgraded components with `downgradeModule()` (#27217) Previously, nested downgraded components would not be created/destroyed inside the Angular zone (as they should) and they would not be wired up correctly for change detection. This commit ensures that ngUpgrade correctly detects whether this is an ngUpgradeLite app (i.e. one using `downgradeModule()` instead of `UpgradeModule`) and appropriately handles components, even if they are nested inside other downgraded components. Fixes #22581 Closes #22869 Closes #27083 PR Close #27217 --- .../upgrade/src/common/downgrade_component.ts | 8 +- .../integration/downgrade_module_spec.ts | 123 ++++++++++++++++++ 2 files changed, 126 insertions(+), 5 deletions(-) diff --git a/packages/upgrade/src/common/downgrade_component.ts b/packages/upgrade/src/common/downgrade_component.ts index 7f34c5d2a2..e1539f06f9 100644 --- a/packages/upgrade/src/common/downgrade_component.ts +++ b/packages/upgrade/src/common/downgrade_component.ts @@ -86,8 +86,9 @@ export function downgradeComponent(info: { // NOTE: This is not needed, when using `UpgradeModule`, because `$digest()` will be run // inside the Angular zone (except if explicitly escaped, in which case we shouldn't // force it back in). - let isNgUpgradeLite = false; - let wrapCallback = (cb: () => T) => cb; + const isNgUpgradeLite = getUpgradeAppType($injector) === UpgradeAppType.Lite; + const wrapCallback: (cb: () => T) => typeof cb = + !isNgUpgradeLite ? cb => cb : cb => () => NgZone.isInAngularZone() ? cb() : ngZone.run(cb); let ngZone: NgZone; return { @@ -112,7 +113,6 @@ export function downgradeComponent(info: { validateInjectionKey($injector, downgradedModule, lazyModuleRefKey, attemptedAction); const lazyModuleRef = $injector.get(lazyModuleRefKey) as LazyModuleRef; - isNgUpgradeLite = getUpgradeAppType($injector) === UpgradeAppType.Lite; parentInjector = lazyModuleRef.injector || lazyModuleRef.promise as Promise; } @@ -149,8 +149,6 @@ export function downgradeComponent(info: { const downgradeFn = !isNgUpgradeLite ? doDowngrade : (injector: Injector) => { if (!ngZone) { ngZone = injector.get(NgZone); - wrapCallback = (cb: () => T) => () => - NgZone.isInAngularZone() ? cb() : ngZone.run(cb); } wrapCallback(() => doDowngrade(injector))(); diff --git a/packages/upgrade/test/static/integration/downgrade_module_spec.ts b/packages/upgrade/test/static/integration/downgrade_module_spec.ts index a1b0455aaf..2c206c4e5e 100644 --- a/packages/upgrade/test/static/integration/downgrade_module_spec.ts +++ b/packages/upgrade/test/static/integration/downgrade_module_spec.ts @@ -322,6 +322,69 @@ withEachNg1Version(() => { }); })); + fixmeIvy('FW-714: ng1 projected content is not being rendered') + .it('should create and destroy nested, asynchronously instantiated components inside the Angular zone', + async(() => { + let createdInTheZone = false; + let destroyedInTheZone = false; + + @Component({ + selector: 'test', + template: '', + }) + class TestComponent implements OnDestroy { + constructor() { createdInTheZone = NgZone.isInAngularZone(); } + ngOnDestroy() { destroyedInTheZone = NgZone.isInAngularZone(); } + } + + @Component({ + selector: 'wrapper', + template: '', + }) + class WrapperComponent { + } + + @NgModule({ + declarations: [TestComponent, WrapperComponent], + entryComponents: [TestComponent, WrapperComponent], + imports: [BrowserModule], + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const bootstrapFn = (extraProviders: StaticProvider[]) => + platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module); + const lazyModuleName = downgradeModule(bootstrapFn); + const ng1Module = + angular.module('ng1', [lazyModuleName]) + .directive( + 'test', downgradeComponent({component: TestComponent, propagateDigest})) + .directive( + 'wrapper', + downgradeComponent({component: WrapperComponent, propagateDigest})); + + // Important: `ng-if` makes `` render asynchronously. + const element = html(''); + const $injector = angular.bootstrap(element, [ng1Module.name]); + const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; + + // Wait for the module to be bootstrapped. + setTimeout(() => { + // Create nested component asynchronously. + expect(createdInTheZone).toBe(false); + + $rootScope.$apply('showNg2 = true'); + expect(createdInTheZone).toBe(true); + + // Destroy nested component asynchronously. + expect(destroyedInTheZone).toBe(false); + + $rootScope.$apply('showNg2 = false'); + expect(destroyedInTheZone).toBe(true); + }); + })); + it('should wire up the component for change detection', async(() => { @Component( {selector: 'ng2', template: '{{ count }}'}) @@ -364,6 +427,66 @@ withEachNg1Version(() => { }); })); + fixmeIvy('FW-714: ng1 projected content is not being rendered') + .it('should wire up nested, asynchronously instantiated components for change detection', + async(() => { + @Component({ + selector: 'test', + template: '{{ count }}' + }) + class TestComponent { + count = 0; + increment() { ++this.count; } + } + + @Component({ + selector: 'wrapper', + template: '', + }) + class WrapperComponent { + } + + @NgModule({ + declarations: [TestComponent, WrapperComponent], + entryComponents: [TestComponent, WrapperComponent], + imports: [BrowserModule], + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const bootstrapFn = (extraProviders: StaticProvider[]) => + platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module); + const lazyModuleName = downgradeModule(bootstrapFn); + const ng1Module = + angular.module('ng1', [lazyModuleName]) + .directive( + 'test', downgradeComponent({component: TestComponent, propagateDigest})) + .directive( + 'wrapper', + downgradeComponent({component: WrapperComponent, propagateDigest})); + + // Important: `ng-if` makes `` render asynchronously. + const element = html(''); + const $injector = angular.bootstrap(element, [ng1Module.name]); + const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; + + // Wait for the module to be bootstrapped. + setTimeout(() => { + // Create nested component asynchronously. + $rootScope.$apply('showNg2 = true'); + const button = element.querySelector('button') !; + + expect(element.textContent).toBe('0'); + + button.click(); + expect(element.textContent).toBe('1'); + + button.click(); + expect(element.textContent).toBe('2'); + }); + })); + fixmeIvy('FW-715: ngOnChanges being called a second time unexpectedly') .fixmeIvy('FW-714: ng1 projected content is not being rendered') .it('should run the lifecycle hooks in the correct order', async(() => {