From a5b61e1956792c8848bb86a3bf576a0b90776390 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 7 Dec 2020 19:55:34 +0200 Subject: [PATCH 1/4] refactor(upgrade): remove unused parameters/properties/variables This commit removes some unused parameters, properties and variables in various `@angular/upgrade` functions. --- packages/upgrade/src/common/src/downgrade_component.ts | 4 ++-- .../upgrade/src/common/src/downgrade_component_adapter.ts | 7 +++---- packages/upgrade/src/common/src/upgrade_helper.ts | 3 +-- .../src/common/test/downgrade_component_adapter_spec.ts | 5 ++--- 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/upgrade/src/common/src/downgrade_component.ts b/packages/upgrade/src/common/src/downgrade_component.ts index bb2f00fce0b41..b0949e3d8583e 100644 --- a/packages/upgrade/src/common/src/downgrade_component.ts +++ b/packages/upgrade/src/common/src/downgrade_component.ts @@ -174,8 +174,8 @@ export function downgradeComponent(info: { const injectorPromise = new ParentInjectorPromise(element); const facade = new DowngradeComponentAdapter( - element, attrs, scope, ngModel, injector, $injector, $compile, $parse, - componentFactory, wrapCallback); + element, attrs, scope, ngModel, injector, $compile, $parse, componentFactory, + wrapCallback); const projectableNodes = facade.compileContents(); facade.createComponent(projectableNodes); diff --git a/packages/upgrade/src/common/src/downgrade_component_adapter.ts b/packages/upgrade/src/common/src/downgrade_component_adapter.ts index 152df408eb442..91b84ab676bc2 100644 --- a/packages/upgrade/src/common/src/downgrade_component_adapter.ts +++ b/packages/upgrade/src/common/src/downgrade_component_adapter.ts @@ -8,7 +8,7 @@ import {ApplicationRef, ChangeDetectorRef, ComponentFactory, ComponentRef, EventEmitter, Injector, OnChanges, SimpleChange, SimpleChanges, StaticProvider, Testability, TestabilityRegistry, Type} from '@angular/core'; -import {IAttributes, IAugmentedJQuery, ICompileService, IInjectorService, INgModelController, IParseService, IScope} from './angular1'; +import {IAttributes, IAugmentedJQuery, ICompileService, INgModelController, IParseService, IScope} from './angular1'; import {PropertyBinding} from './component_info'; import {$SCOPE} from './constants'; import {getTypeName, hookupNgModel, strictEquals} from './util'; @@ -33,8 +33,8 @@ export class DowngradeComponentAdapter { constructor( private element: IAugmentedJQuery, private attrs: IAttributes, private scope: IScope, private ngModel: INgModelController, private parentInjector: Injector, - private $injector: IInjectorService, private $compile: ICompileService, - private $parse: IParseService, private componentFactory: ComponentFactory, + private $compile: ICompileService, private $parse: IParseService, + private componentFactory: ComponentFactory, private wrapCallback: (cb: () => T) => () => T) { this.componentScope = scope.$new(); } @@ -250,7 +250,6 @@ export class DowngradeComponentAdapter { */ export function groupNodesBySelector(ngContentSelectors: string[], nodes: Node[]): Node[][] { const projectableNodes: Node[][] = []; - let wildcardNgContentIndex: number; for (let i = 0, ii = ngContentSelectors.length; i < ii; ++i) { projectableNodes[i] = []; diff --git a/packages/upgrade/src/common/src/upgrade_helper.ts b/packages/upgrade/src/common/src/upgrade_helper.ts index f5414cf8ecedb..0d520eb9e6a4b 100644 --- a/packages/upgrade/src/common/src/upgrade_helper.ts +++ b/packages/upgrade/src/common/src/upgrade_helper.ts @@ -41,8 +41,7 @@ export class UpgradeHelper { private readonly $controller: IControllerService; constructor( - private injector: Injector, private name: string, elementRef: ElementRef, - directive?: IDirective) { + injector: Injector, private name: string, elementRef: ElementRef, directive?: IDirective) { this.$injector = injector.get($INJECTOR); this.$compile = this.$injector.get($COMPILE); this.$controller = this.$injector.get($CONTROLLER); diff --git a/packages/upgrade/src/common/test/downgrade_component_adapter_spec.ts b/packages/upgrade/src/common/test/downgrade_component_adapter_spec.ts index e59470ca88c10..0a2300a2e79da 100644 --- a/packages/upgrade/src/common/test/downgrade_component_adapter_spec.ts +++ b/packages/upgrade/src/common/test/downgrade_component_adapter_spec.ts @@ -131,7 +131,6 @@ withEachNg1Version(() => { let scope: angular.IScope; // mock let ngModel = undefined as any; let parentInjector: Injector; // testbed - let $injector = undefined as any; let $compile = undefined as any; let $parse = undefined as any; let componentFactory: ComponentFactory; // testbed @@ -166,8 +165,8 @@ withEachNg1Version(() => { parentInjector = TestBed; return new DowngradeComponentAdapter( - element, attrs, scope, ngModel, parentInjector, $injector, $compile, $parse, - componentFactory, wrapCallback); + element, attrs, scope, ngModel, parentInjector, $compile, $parse, componentFactory, + wrapCallback); } beforeEach(() => { From 0fe42339410ff1b54353ff27dfa03477fcd9a09a Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 7 Dec 2020 19:55:34 +0200 Subject: [PATCH 2/4] fix(upgrade): avoid memory leak when removing downgraded components Previously, due to the way the AngularJS and Angular clean-up processes interfere with each other when removing an AngularJS element that contains a downgraded Angular component, the data associated with the host element of the downgraded component was not removed. This data was kept in an internal AngularJS cache, which prevented the element and component instance from being garbage-collected, leading to memory leaks. This commit fixes this by ensuring the element data is explicitly removed when cleaning up a downgraded component. NOTE: This is essentially the equivalent of #26209 but for downgraded (instead of upgraded) components. Fixes #39911 Closes #39921 --- packages/upgrade/src/common/src/angular1.ts | 1 + .../common/src/downgrade_component_adapter.ts | 31 ++++++++- .../upgrade/src/dynamic/test/upgrade_spec.ts | 28 ++++++++- .../integration/downgrade_component_spec.ts | 21 ++++++- .../test/integration/downgrade_module_spec.ts | 63 +++++++++++++++++++ 5 files changed, 139 insertions(+), 5 deletions(-) diff --git a/packages/upgrade/src/common/src/angular1.ts b/packages/upgrade/src/common/src/angular1.ts index b62bf6b718328..b4a8b94d12866 100644 --- a/packages/upgrade/src/common/src/angular1.ts +++ b/packages/upgrade/src/common/src/angular1.ts @@ -126,6 +126,7 @@ export type IAugmentedJQuery = Node[]&{ data?: (name: string, value?: any) => any; text?: () => string; inheritedData?: (name: string, value?: any) => any; + children?: () => IAugmentedJQuery; contents?: () => IAugmentedJQuery; parent?: () => IAugmentedJQuery; empty?: () => void; diff --git a/packages/upgrade/src/common/src/downgrade_component_adapter.ts b/packages/upgrade/src/common/src/downgrade_component_adapter.ts index 91b84ab676bc2..a0d5bc8c0d46c 100644 --- a/packages/upgrade/src/common/src/downgrade_component_adapter.ts +++ b/packages/upgrade/src/common/src/downgrade_component_adapter.ts @@ -8,7 +8,7 @@ import {ApplicationRef, ChangeDetectorRef, ComponentFactory, ComponentRef, EventEmitter, Injector, OnChanges, SimpleChange, SimpleChanges, StaticProvider, Testability, TestabilityRegistry, Type} from '@angular/core'; -import {IAttributes, IAugmentedJQuery, ICompileService, INgModelController, IParseService, IScope} from './angular1'; +import {element as angularElement, IAttributes, IAugmentedJQuery, ICompileService, INgModelController, IParseService, IScope} from './angular1'; import {PropertyBinding} from './component_info'; import {$SCOPE} from './constants'; import {getTypeName, hookupNgModel, strictEquals} from './util'; @@ -216,11 +216,38 @@ export class DowngradeComponentAdapter { const destroyComponentRef = this.wrapCallback(() => this.componentRef.destroy()); let destroyed = false; - this.element.on!('$destroy', () => this.componentScope.$destroy()); + this.element.on!('$destroy', () => { + // The `$destroy` event may have been triggered by the `cleanData()` call in the + // `componentScope` `$destroy` handler below. In that case, we don't want to call + // `componentScope.$destroy()` again. + if (!destroyed) this.componentScope.$destroy(); + }); this.componentScope.$on('$destroy', () => { if (!destroyed) { destroyed = true; testabilityRegistry.unregisterApplication(this.componentRef.location.nativeElement); + + // The `componentScope` might be getting destroyed, because an ancestor element is being + // removed/destroyed. If that is the case, jqLite/jQuery would normally invoke `cleanData()` + // on the removed element and all descendants. + // https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/jqLite.js#L349-L355 + // https://github.com/jquery/jquery/blob/6984d1747623dbc5e87fd6c261a5b6b1628c107c/src/manipulation.js#L182 + // + // Here, however, `destroyComponentRef()` may under some circumstances remove the element + // from the DOM and therefore it will no longer be a descendant of the removed element when + // `cleanData()` is called. This would result in a memory leak, because the element's data + // and event handlers (and all objects directly or indirectly referenced by them) would be + // retained. + // + // To ensure the element is always properly cleaned up, we manually call `cleanData()` on + // this element and its descendants before destroying the `ComponentRef`. + // + // NOTE: + // `cleanData()` also will invoke the AngularJS `$destroy` event on the element: + // https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/Angular.js#L1932-L1945 + angularElement.cleanData(this.element); + angularElement.cleanData(this.element.children!()); + destroyComponentRef(); } }); diff --git a/packages/upgrade/src/dynamic/test/upgrade_spec.ts b/packages/upgrade/src/dynamic/test/upgrade_spec.ts index 9ab7ba7ff8072..42c0a299ceb06 100644 --- a/packages/upgrade/src/dynamic/test/upgrade_spec.ts +++ b/packages/upgrade/src/dynamic/test/upgrade_spec.ts @@ -678,7 +678,7 @@ withEachNg1Version(() => { }; }); - @Component({selector: 'ng2', template: 'test'}) + @Component({selector: 'ng2', template: 'test'}) class Ng2 { ngOnDestroy() { onDestroyed.emit('destroyed'); @@ -695,8 +695,32 @@ withEachNg1Version(() => { ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2)); const element = html(''); adapter.bootstrap(element, ['ng1']).ready((ref) => { + const ng2Element = angular.element(element.querySelector('ng2') as Element); + const ng2Children = (ng2Element as any).children!(); + let ng2ElementDestroyed = false; + let ng2ChildrenDestroyed = false; + + ng2Element.data!('foo', 1); + ng2Children.data!('bar', 2); + ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); + ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); + + expect(element.textContent).toContain('test'); + expect(ng2Element.data!('foo')).toBe(1); + expect(ng2Children.data!('bar')).toBe(2); + expect(ng2ElementDestroyed).toBe(false); + expect(ng2ChildrenDestroyed).toBe(false); + onDestroyed.subscribe(() => { - ref.dispose(); + setTimeout(() => { + expect(element.textContent).not.toContain('test'); + expect(ng2Element.data!('foo')).toBeUndefined(); + expect(ng2Children.data!('baz')).toBeUndefined(); + expect(ng2ElementDestroyed).toBe(true); + expect(ng2ChildrenDestroyed).toBe(true); + + ref.dispose(); + }); }); }); })); diff --git a/packages/upgrade/static/test/integration/downgrade_component_spec.ts b/packages/upgrade/static/test/integration/downgrade_component_spec.ts index e5ad0978b4f46..b39e4aa9548e0 100644 --- a/packages/upgrade/static/test/integration/downgrade_component_spec.ts +++ b/packages/upgrade/static/test/integration/downgrade_component_spec.ts @@ -536,7 +536,7 @@ withEachNg1Version(() => { it('should properly run cleanup when ng1 directive is destroyed', waitForAsync(() => { let destroyed = false; - @Component({selector: 'ng2', template: 'test'}) + @Component({selector: 'ng2', template: 'test'}) class Ng2Component implements OnDestroy { ngOnDestroy() { destroyed = true; @@ -563,14 +563,33 @@ withEachNg1Version(() => { platformBrowserDynamic().bootstrapModule(Ng2Module).then((ref) => { const adapter = ref.injector.get(UpgradeModule) as UpgradeModule; adapter.bootstrap(element, [ng1Module.name]); + + const ng2Element = angular.element(element.querySelector('ng2') as Element); + const ng2Children = (ng2Element as any).children!(); + let ng2ElementDestroyed = false; + let ng2ChildrenDestroyed = false; + + ng2Element.data!('foo', 1); + ng2Children.data!('bar', 2); + ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); + ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); + expect(element.textContent).toContain('test'); expect(destroyed).toBe(false); + expect(ng2Element.data!('foo')).toBe(1); + expect(ng2Children.data!('bar')).toBe(2); + expect(ng2ElementDestroyed).toBe(false); + expect(ng2ChildrenDestroyed).toBe(false); const $rootScope = adapter.$injector.get('$rootScope'); $rootScope.$apply('destroyIt = true'); expect(element.textContent).not.toContain('test'); expect(destroyed).toBe(true); + expect(ng2Element.data!('foo')).toBeUndefined(); + expect(ng2Children.data!('baz')).toBeUndefined(); + expect(ng2ElementDestroyed).toBe(true); + expect(ng2ChildrenDestroyed).toBe(true); }); })); diff --git a/packages/upgrade/static/test/integration/downgrade_module_spec.ts b/packages/upgrade/static/test/integration/downgrade_module_spec.ts index 020081077fea7..33845b2be3619 100644 --- a/packages/upgrade/static/test/integration/downgrade_module_spec.ts +++ b/packages/upgrade/static/test/integration/downgrade_module_spec.ts @@ -1187,6 +1187,69 @@ withEachNg1Version(() => { }); })); + it('should properly run cleanup when a downgraded component is destroyed', + waitForAsync(() => { + let destroyed = false; + + @Component({selector: 'ng2', template: 'test'}) + class Ng2Component implements OnDestroy { + ngOnDestroy() { + destroyed = true; + } + } + + @NgModule({ + declarations: [Ng2Component], + entryComponents: [Ng2Component], + imports: [BrowserModule], + }) + class Ng2Module { + ngDoBootstrap() {} + } + + const bootstrapFn = (extraProviders: StaticProvider[]) => + platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module); + const lazyModuleName = downgradeModule(bootstrapFn); + const ng1Module = + angular.module_('ng1', [lazyModuleName]) + .directive( + 'ng2', downgradeComponent({component: Ng2Component, propagateDigest})); + + const element = html('
'); + const $injector = angular.bootstrap(element, [ng1Module.name]); + const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; + + setTimeout(() => { // Wait for the module to be bootstrapped. + setTimeout(() => { // Wait for the hostView to be attached (during the `$digest`). + const ng2Element = angular.element(element.querySelector('ng2') as Element); + const ng2Children = (ng2Element as any).children!(); + let ng2ElementDestroyed = false; + let ng2ChildrenDestroyed = false; + + ng2Element.data!('foo', 1); + ng2Children.data!('bar', 2); + ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); + ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); + + expect(element.textContent).toContain('test'); + expect(destroyed).toBe(false); + expect(ng2Element.data!('foo')).toBe(1); + expect(ng2Children.data!('bar')).toBe(2); + expect(ng2ElementDestroyed).toBe(false); + expect(ng2ChildrenDestroyed).toBe(false); + + $rootScope.$apply('hideNg2 = true'); + + expect(element.textContent).not.toContain('test'); + expect(destroyed).toBe(true); + expect(ng2Element.data!('foo')).toBeUndefined(); + expect(ng2Children.data!('baz')).toBeUndefined(); + expect(ng2ElementDestroyed).toBe(true); + expect(ng2ChildrenDestroyed).toBe(true); + }); + }); + })); + it('should only retrieve the Angular zone once (and cache it for later use)', fakeAsync(() => { let count = 0; From b3f23b48413094fe5583507192b71c7cdd307489 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 7 Dec 2020 20:10:56 +0200 Subject: [PATCH 3/4] fixup! fix(upgrade): avoid memory leak when removing downgraded components --- .../upgrade/src/dynamic/test/upgrade_spec.ts | 37 ++++++++----------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/packages/upgrade/src/dynamic/test/upgrade_spec.ts b/packages/upgrade/src/dynamic/test/upgrade_spec.ts index 42c0a299ceb06..374f5b696d3d5 100644 --- a/packages/upgrade/src/dynamic/test/upgrade_spec.ts +++ b/packages/upgrade/src/dynamic/test/upgrade_spec.ts @@ -665,23 +665,16 @@ withEachNg1Version(() => { it('should properly run cleanup when ng1 directive is destroyed', waitForAsync(() => { const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module)); const ng1Module = angular.module_('ng1', []); - const onDestroyed: EventEmitter = new EventEmitter(); + let ng2ComponentDestroyed = false; - ng1Module.directive('ng1', () => { - return { - template: '
', - controller: function($rootScope: any, $timeout: Function) { - $timeout(() => { - $rootScope.destroyIt = true; - }); - } - }; - }); + ng1Module.directive('ng1', () => ({ + template: '
', + })); @Component({selector: 'ng2', template: 'test'}) class Ng2 { ngOnDestroy() { - onDestroyed.emit('destroyed'); + ng2ComponentDestroyed = true; } } @@ -710,18 +703,18 @@ withEachNg1Version(() => { expect(ng2Children.data!('bar')).toBe(2); expect(ng2ElementDestroyed).toBe(false); expect(ng2ChildrenDestroyed).toBe(false); + expect(ng2ComponentDestroyed).toBe(false); - onDestroyed.subscribe(() => { - setTimeout(() => { - expect(element.textContent).not.toContain('test'); - expect(ng2Element.data!('foo')).toBeUndefined(); - expect(ng2Children.data!('baz')).toBeUndefined(); - expect(ng2ElementDestroyed).toBe(true); - expect(ng2ChildrenDestroyed).toBe(true); + ref.ng1RootScope.$apply('destroyIt = true'); - ref.dispose(); - }); - }); + expect(element.textContent).not.toContain('test'); + expect(ng2Element.data!('foo')).toBeUndefined(); + expect(ng2Children.data!('baz')).toBeUndefined(); + expect(ng2ElementDestroyed).toBe(true); + expect(ng2ChildrenDestroyed).toBe(true); + expect(ng2ComponentDestroyed).toBe(true); + + ref.dispose(); }); })); From 7d18ca30c8311646530da063d3c08579ff90da82 Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Mon, 7 Dec 2020 20:37:15 +0200 Subject: [PATCH 4/4] fixup! fix(upgrade): avoid memory leak when removing downgraded components --- .../common/src/downgrade_component_adapter.ts | 2 +- .../upgrade/src/dynamic/test/upgrade_spec.ts | 34 ++++++------ .../integration/downgrade_component_spec.ts | 30 ++++++----- .../test/integration/downgrade_module_spec.ts | 54 +++++++++---------- 4 files changed, 62 insertions(+), 58 deletions(-) diff --git a/packages/upgrade/src/common/src/downgrade_component_adapter.ts b/packages/upgrade/src/common/src/downgrade_component_adapter.ts index a0d5bc8c0d46c..5022a58aaca22 100644 --- a/packages/upgrade/src/common/src/downgrade_component_adapter.ts +++ b/packages/upgrade/src/common/src/downgrade_component_adapter.ts @@ -246,7 +246,7 @@ export class DowngradeComponentAdapter { // `cleanData()` also will invoke the AngularJS `$destroy` event on the element: // https://github.com/angular/angular.js/blob/2e72ea13fa98bebf6ed4b5e3c45eaf5f990ed16f/src/Angular.js#L1932-L1945 angularElement.cleanData(this.element); - angularElement.cleanData(this.element.children!()); + angularElement.cleanData((this.element[0] as Element).querySelectorAll('*')); destroyComponentRef(); } diff --git a/packages/upgrade/src/dynamic/test/upgrade_spec.ts b/packages/upgrade/src/dynamic/test/upgrade_spec.ts index 374f5b696d3d5..3d33cc59be02d 100644 --- a/packages/upgrade/src/dynamic/test/upgrade_spec.ts +++ b/packages/upgrade/src/dynamic/test/upgrade_spec.ts @@ -668,10 +668,10 @@ withEachNg1Version(() => { let ng2ComponentDestroyed = false; ng1Module.directive('ng1', () => ({ - template: '
', - })); + template: '
', + })); - @Component({selector: 'ng2', template: 'test'}) + @Component({selector: 'ng2', template: '
  • test1
  • test2
'}) class Ng2 { ngOnDestroy() { ng2ComponentDestroyed = true; @@ -689,29 +689,31 @@ withEachNg1Version(() => { const element = html(''); adapter.bootstrap(element, ['ng1']).ready((ref) => { const ng2Element = angular.element(element.querySelector('ng2') as Element); - const ng2Children = (ng2Element as any).children!(); + const ng2Descendants = + Array.from(element.querySelectorAll('ng2 li')).map(angular.element); let ng2ElementDestroyed = false; - let ng2ChildrenDestroyed = false; + let ng2DescendantsDestroyed = ng2Descendants.map(() => false); - ng2Element.data!('foo', 1); - ng2Children.data!('bar', 2); + ng2Element.data!('test', 42); + ng2Descendants.forEach((elem, i) => elem.data!('test', i)); ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); - ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); + ng2Descendants.forEach( + (elem, i) => elem.on!('$destroy', () => ng2DescendantsDestroyed[i] = true)); - expect(element.textContent).toContain('test'); - expect(ng2Element.data!('foo')).toBe(1); - expect(ng2Children.data!('bar')).toBe(2); + expect(element.textContent).toBe('test1test2'); + expect(ng2Element.data!('test')).toBe(42); + ng2Descendants.forEach((elem, i) => expect(elem.data!('test')).toBe(i)); expect(ng2ElementDestroyed).toBe(false); - expect(ng2ChildrenDestroyed).toBe(false); + expect(ng2DescendantsDestroyed).toEqual([false, false]); expect(ng2ComponentDestroyed).toBe(false); ref.ng1RootScope.$apply('destroyIt = true'); - expect(element.textContent).not.toContain('test'); - expect(ng2Element.data!('foo')).toBeUndefined(); - expect(ng2Children.data!('baz')).toBeUndefined(); + expect(element.textContent).toBe(''); + expect(ng2Element.data!('test')).toBeUndefined(); + ng2Descendants.forEach(elem => expect(elem.data!('test')).toBeUndefined()); expect(ng2ElementDestroyed).toBe(true); - expect(ng2ChildrenDestroyed).toBe(true); + expect(ng2DescendantsDestroyed).toEqual([true, true]); expect(ng2ComponentDestroyed).toBe(true); ref.dispose(); diff --git a/packages/upgrade/static/test/integration/downgrade_component_spec.ts b/packages/upgrade/static/test/integration/downgrade_component_spec.ts index b39e4aa9548e0..ec88d423d158a 100644 --- a/packages/upgrade/static/test/integration/downgrade_component_spec.ts +++ b/packages/upgrade/static/test/integration/downgrade_component_spec.ts @@ -536,7 +536,7 @@ withEachNg1Version(() => { it('should properly run cleanup when ng1 directive is destroyed', waitForAsync(() => { let destroyed = false; - @Component({selector: 'ng2', template: 'test'}) + @Component({selector: 'ng2', template: '
  • test1
  • test2
'}) class Ng2Component implements OnDestroy { ngOnDestroy() { destroyed = true; @@ -565,31 +565,33 @@ withEachNg1Version(() => { adapter.bootstrap(element, [ng1Module.name]); const ng2Element = angular.element(element.querySelector('ng2') as Element); - const ng2Children = (ng2Element as any).children!(); + const ng2Descendants = + Array.from(element.querySelectorAll('ng2 li')).map(angular.element); let ng2ElementDestroyed = false; - let ng2ChildrenDestroyed = false; + let ng2DescendantsDestroyed = [false, false]; - ng2Element.data!('foo', 1); - ng2Children.data!('bar', 2); + ng2Element.data!('test', 42); + ng2Descendants.forEach((elem, i) => elem.data!('test', i)); ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); - ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); + ng2Descendants.forEach( + (elem, i) => elem.on!('$destroy', () => ng2DescendantsDestroyed[i] = true)); - expect(element.textContent).toContain('test'); + expect(element.textContent).toBe('test1test2'); expect(destroyed).toBe(false); - expect(ng2Element.data!('foo')).toBe(1); - expect(ng2Children.data!('bar')).toBe(2); + expect(ng2Element.data!('test')).toBe(42); + ng2Descendants.forEach((elem, i) => expect(elem.data!('test')).toBe(i)); expect(ng2ElementDestroyed).toBe(false); - expect(ng2ChildrenDestroyed).toBe(false); + expect(ng2DescendantsDestroyed).toEqual([false, false]); const $rootScope = adapter.$injector.get('$rootScope'); $rootScope.$apply('destroyIt = true'); - expect(element.textContent).not.toContain('test'); + expect(element.textContent).toBe(''); expect(destroyed).toBe(true); - expect(ng2Element.data!('foo')).toBeUndefined(); - expect(ng2Children.data!('baz')).toBeUndefined(); + expect(ng2Element.data!('test')).toBeUndefined(); + ng2Descendants.forEach(elem => expect(elem.data!('test')).toBeUndefined()); expect(ng2ElementDestroyed).toBe(true); - expect(ng2ChildrenDestroyed).toBe(true); + expect(ng2DescendantsDestroyed).toEqual([true, true]); }); })); diff --git a/packages/upgrade/static/test/integration/downgrade_module_spec.ts b/packages/upgrade/static/test/integration/downgrade_module_spec.ts index 33845b2be3619..cc34ec57b7f5f 100644 --- a/packages/upgrade/static/test/integration/downgrade_module_spec.ts +++ b/packages/upgrade/static/test/integration/downgrade_module_spec.ts @@ -1191,7 +1191,7 @@ withEachNg1Version(() => { waitForAsync(() => { let destroyed = false; - @Component({selector: 'ng2', template: 'test'}) + @Component({selector: 'ng2', template: '
  • test1
  • test2
'}) class Ng2Component implements OnDestroy { ngOnDestroy() { destroyed = true; @@ -1219,34 +1219,34 @@ withEachNg1Version(() => { const $injector = angular.bootstrap(element, [ng1Module.name]); const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService; - setTimeout(() => { // Wait for the module to be bootstrapped. - setTimeout(() => { // Wait for the hostView to be attached (during the `$digest`). - const ng2Element = angular.element(element.querySelector('ng2') as Element); - const ng2Children = (ng2Element as any).children!(); - let ng2ElementDestroyed = false; - let ng2ChildrenDestroyed = false; - - ng2Element.data!('foo', 1); - ng2Children.data!('bar', 2); - ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); - ng2Children.on!('$destroy', () => ng2ChildrenDestroyed = true); - - expect(element.textContent).toContain('test'); - expect(destroyed).toBe(false); - expect(ng2Element.data!('foo')).toBe(1); - expect(ng2Children.data!('bar')).toBe(2); - expect(ng2ElementDestroyed).toBe(false); - expect(ng2ChildrenDestroyed).toBe(false); + setTimeout(() => { // Wait for the module to be bootstrapped. + const ng2Element = angular.element(element.querySelector('ng2') as Element); + const ng2Descendants = + Array.from(element.querySelectorAll('ng2 li')).map(angular.element); + let ng2ElementDestroyed = false; + let ng2DescendantsDestroyed = [false, false]; + + ng2Element.data!('test', 42); + ng2Descendants.forEach((elem, i) => elem.data!('test', i)); + ng2Element.on!('$destroy', () => ng2ElementDestroyed = true); + ng2Descendants.forEach( + (elem, i) => elem.on!('$destroy', () => ng2DescendantsDestroyed[i] = true)); + + expect(element.textContent).toBe('test1test2'); + expect(destroyed).toBe(false); + expect(ng2Element.data!('test')).toBe(42); + ng2Descendants.forEach((elem, i) => expect(elem.data!('test')).toBe(i)); + expect(ng2ElementDestroyed).toBe(false); + expect(ng2DescendantsDestroyed).toEqual([false, false]); - $rootScope.$apply('hideNg2 = true'); + $rootScope.$apply('hideNg2 = true'); - expect(element.textContent).not.toContain('test'); - expect(destroyed).toBe(true); - expect(ng2Element.data!('foo')).toBeUndefined(); - expect(ng2Children.data!('baz')).toBeUndefined(); - expect(ng2ElementDestroyed).toBe(true); - expect(ng2ChildrenDestroyed).toBe(true); - }); + expect(element.textContent).toBe(''); + expect(destroyed).toBe(true); + expect(ng2Element.data!('test')).toBeUndefined(); + ng2Descendants.forEach(elem => expect(elem.data!('test')).toBeUndefined()); + expect(ng2ElementDestroyed).toBe(true); + expect(ng2DescendantsDestroyed).toEqual([true, true]); }); }));