fix(upgrade): properly destroy upgraded component elements and descen…

…dants Fixes #26208
angular · Oct 2, 2018 · 528fe26 · 528fe26
1 parent a2878b0
commit 528fe26
Show file tree

Hide file tree

Showing 3 changed files with 205 additions and 1 deletion.
diff --git a/packages/upgrade/src/common/angular1.ts b/packages/upgrade/src/common/angular1.ts
@@ -287,4 +287,7 @@ export const resumeBootstrap: typeof angular.resumeBootstrap = () => angular.res
 
 export const getTestability: typeof angular.getTestability = e => angular.getTestability(e);
 
+export const cleanData: (l: NodeList | Node[]) => void = (nodes) =>
+    (angular.element as any).cleanData(nodes);
+
 export let version = angular.version;
diff --git a/packages/upgrade/src/common/upgrade_helper.ts b/packages/upgrade/src/common/upgrade_helper.ts
@@ -124,7 +124,8 @@ export class UpgradeHelper {
       controllerInstance.$onDestroy();
     }
     $scope.$destroy();
-    this.$element.triggerHandler !('$destroy');
+    angular.cleanData([this.element]);
+    angular.cleanData(this.element.querySelectorAll('*'));
   }
 
   prepareTransclusion(): angular.ILinkFn|undefined {

diff --git a/packages/upgrade/test/static/integration/upgrade_component_spec.ts b/packages/upgrade/test/static/integration/upgrade_component_spec.ts
@@ -3694,6 +3694,206 @@ withEachNg1Version(() => {
            });
          }));
 
+      it('should emit `$destroy` on `$element` descendants', async(() => {
+           const elementDestroyListener = jasmine.createSpy('elementDestroyListener');
+           let ng2ComponentAInstance: Ng2ComponentA;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             controller: class {
+               constructor($element: angular.IAugmentedJQuery) {
+                 $element.contents !().on !('$destroy', elementDestroyListener);
+               }
+             },
+             template: '<div></div>'
+           };
+
+           // Define `Ng1ComponentFacade`
+           @Directive({selector: 'ng1'})
+           class Ng1ComponentFacade extends UpgradeComponent {
+             constructor(elementRef: ElementRef, injector: Injector) {
+               super('ng1', elementRef, injector);
+             }
+           }
+
+           // Define `Ng2Component`
+           @Component({selector: 'ng2A', template: '<ng2B *ngIf="!destroyIt"></ng2B>'})
+           class Ng2ComponentA {
+             destroyIt = false;
+
+             constructor() { ng2ComponentAInstance = this; }
+           }
+
+           @Component({selector: 'ng2B', template: '<ng1></ng1>'})
+           class Ng2ComponentB {
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2A', downgradeComponent({component: Ng2ComponentA}));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [Ng1ComponentFacade, Ng2ComponentA, Ng2ComponentB],
+             entryComponents: [Ng2ComponentA],
+             imports: [BrowserModule, UpgradeModule]
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
+
+           // Bootstrap
+           const element = html(`<ng2-a></ng2-a>`);
+
+           bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
+             expect(elementDestroyListener).not.toHaveBeenCalled();
+
+             ng2ComponentAInstance.destroyIt = true;
+             $digest(adapter);
+
+             expect(elementDestroyListener).toHaveBeenCalledTimes(1);
+           });
+         }));
+
+      it('should clear data on element and descendants`', async(() => {
+           let ng1ComponentElement: angular.IAugmentedJQuery;
+           let ng2ComponentAInstance: Ng2ComponentA;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             controller: class {
+               constructor($element: angular.IAugmentedJQuery) {
+                 $element.data !('test', 1);
+                 $element.contents !().data !('test', 2);
+
+                 ng1ComponentElement = $element;
+               }
+             },
+             template: '<div></div>'
+           };
+
+           // Define `Ng1ComponentFacade`
+           @Directive({selector: 'ng1'})
+           class Ng1ComponentFacade extends UpgradeComponent {
+             constructor(elementRef: ElementRef, injector: Injector) {
+               super('ng1', elementRef, injector);
+             }
+           }
+
+           // Define `Ng2Component`
+           @Component({selector: 'ng2A', template: '<ng2B *ngIf="!destroyIt"></ng2B>'})
+           class Ng2ComponentA {
+             destroyIt = false;
+
+             constructor() { ng2ComponentAInstance = this; }
+           }
+
+           @Component({selector: 'ng2B', template: '<ng1></ng1>'})
+           class Ng2ComponentB {
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2A', downgradeComponent({component: Ng2ComponentA}));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [Ng1ComponentFacade, Ng2ComponentA, Ng2ComponentB],
+             entryComponents: [Ng2ComponentA],
+             imports: [BrowserModule, UpgradeModule]
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
+
+           // Bootstrap
+           const element = html(`<ng2-a></ng2-a>`);
+
+           bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
+             expect(ng1ComponentElement.data !('test')).toBe(1);
+             expect(ng1ComponentElement.contents !().data !('test')).toBe(2);
+
+             ng2ComponentAInstance.destroyIt = true;
+             $digest(adapter);
+
+             expect(ng1ComponentElement.data !('test')).toBeUndefined();
+             expect(ng1ComponentElement.contents !().data !('test')).toBeUndefined();
+           });
+         }));
+
+      it('should clear dom listeners on element and descendants`', async(() => {
+           const elementClickListener = jasmine.createSpy('elementClickListener');
+           const descendantClickListener = jasmine.createSpy('descendantClickListener');
+           let ng1DescendantElement: angular.IAugmentedJQuery;
+           let ng2ComponentAInstance: Ng2ComponentA;
+
+           // Define `ng1Component`
+           const ng1Component: angular.IComponent = {
+             controller: class {
+               constructor($element: angular.IAugmentedJQuery) {
+                 ng1DescendantElement = $element.contents !();
+
+                 $element.on !('click', elementClickListener);
+                 ng1DescendantElement.on !('click', descendantClickListener);
+               }
+             },
+             template: '<div></div>'
+           };
+
+           // Define `Ng1ComponentFacade`
+           @Directive({selector: 'ng1'})
+           class Ng1ComponentFacade extends UpgradeComponent {
+             constructor(elementRef: ElementRef, injector: Injector) {
+               super('ng1', elementRef, injector);
+             }
+           }
+
+           // Define `Ng2Component`
+           @Component({selector: 'ng2A', template: '<ng2B *ngIf="!destroyIt"></ng2B>'})
+           class Ng2ComponentA {
+             destroyIt = false;
+
+             constructor() { ng2ComponentAInstance = this; }
+           }
+
+           @Component({selector: 'ng2B', template: '<ng1></ng1>'})
+           class Ng2ComponentB {
+           }
+
+           // Define `ng1Module`
+           const ng1Module = angular.module('ng1Module', [])
+                                 .component('ng1', ng1Component)
+                                 .directive('ng2A', downgradeComponent({component: Ng2ComponentA}));
+
+           // Define `Ng2Module`
+           @NgModule({
+             declarations: [Ng1ComponentFacade, Ng2ComponentA, Ng2ComponentB],
+             entryComponents: [Ng2ComponentA],
+             imports: [BrowserModule, UpgradeModule]
+           })
+           class Ng2Module {
+             ngDoBootstrap() {}
+           }
+
+           // Bootstrap
+           const element = html(`<ng2-a></ng2-a>`);
+
+           bootstrap(platformBrowserDynamic(), Ng2Module, element, ng1Module).then(adapter => {
+             (ng1DescendantElement[0] as HTMLElement).click();
+             expect(elementClickListener).toHaveBeenCalledTimes(1);
+             expect(descendantClickListener).toHaveBeenCalledTimes(1);
+
+             ng2ComponentAInstance.destroyIt = true;
+             $digest(adapter);
+
+             (ng1DescendantElement[0] as HTMLElement).click();
+             expect(elementClickListener).toHaveBeenCalledTimes(1);
+             expect(descendantClickListener).toHaveBeenCalledTimes(1);
+           });
+         }));
+
       it('should clean up `$doCheck()` watchers from the parent scope', async(() => {
            let ng2Component: Ng2Component;