Skip to content

Commit

Permalink
fixup! fix(upgrade): avoid memory leak when removing downgraded compo…
Browse files Browse the repository at this point in the history 
…nents
  • Loading branch information
@gkalpak
gkalpak committed Dec 7, 2020
1 parent b3f23b4 commit 7d18ca3
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 58 deletions.
2 changes: 1 addition & 1 deletion packages/upgrade/src/common/src/downgrade_component_adapter.ts
View file
Expand Up @@ -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();
}
Expand Down
34 changes: 18 additions & 16 deletions packages/upgrade/src/dynamic/test/upgrade_spec.ts
View file
Expand Up @@ -668,10 +668,10 @@ withEachNg1Version(() => {
let ng2ComponentDestroyed = false;

ng1Module.directive('ng1', () => ({
template: '<div ng-if="!destroyIt"><ng2></ng2></div>',
}));
template: '<div ng-if="!destroyIt"><ng2></ng2></div>',
}));

@Component({selector: 'ng2', template: '<span>test</span>'})
@Component({selector: 'ng2', template: '<ul><li>test1</li><li>test2</li></ul>'})
class Ng2 {
ngOnDestroy() {
ng2ComponentDestroyed = true;
Expand All @@ -689,29 +689,31 @@ withEachNg1Version(() => {
const element = html('<ng1></ng1>');
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();
Expand Down
30 changes: 16 additions & 14 deletions packages/upgrade/static/test/integration/downgrade_component_spec.ts
View file
Expand Up @@ -536,7 +536,7 @@ withEachNg1Version(() => {

it('should properly run cleanup when ng1 directive is destroyed', waitForAsync(() => {
let destroyed = false;
@Component({selector: 'ng2', template: '<span>test</span>'})
@Component({selector: 'ng2', template: '<ul><li>test1</li><li>test2</li></ul>'})
class Ng2Component implements OnDestroy {
ngOnDestroy() {
destroyed = true;
Expand Down Expand Up @@ -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]);
});
}));

Expand Down
54 changes: 27 additions & 27 deletions packages/upgrade/static/test/integration/downgrade_module_spec.ts
View file
Expand Up @@ -1191,7 +1191,7 @@ withEachNg1Version(() => {
waitForAsync(() => {
let destroyed = false;

@Component({selector: 'ng2', template: '<span>test</span>'})
@Component({selector: 'ng2', template: '<ul><li>test1</li><li>test2</li></ul>'})
class Ng2Component implements OnDestroy {
ngOnDestroy() {
destroyed = true;
Expand Down Expand Up @@ -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]);
});
}));

Expand Down

0 comments on commit 7d18ca3

Please sign in to comment.