Skip to content
Permalink
Browse files

fix(upgrade): upgrade Directive facade should not return different in…

…stance from constructor (#27660)

In ngUpgrade (dynamic) we create a dynamic Angular `Directive` that wraps AngularJS components
that are being upgraded. The constructor of this `Directive` class returns a different instance
than `this`. It is this instance that actually contains the life-cycle hook handlers.

This would break in ivy, since the methods on the prototype of the original class are wired up,
rather than the instance methods. This results in hooks like `ngOnInit` not being called.

This commit refactors the code to extend the inner class that was being returned so that the
prototype chain is correct for both ViewEngine and ivy.

This change resolves a number of failing ivy tests, but also exposes other failures that were
masked by this issue. The tests have been updated accordingly.

(FW-812)

PR Close #27660
  • Loading branch information...
petebacondarwin authored and mhevery committed Dec 13, 2018
1 parent 3444aee commit c986d3dcf4ba33cec6a448d17d1d344e8641bde3
Showing with 1,108 additions and 1,179 deletions.
  1. +8 −17 packages/upgrade/src/dynamic/upgrade_ng1_adapter.ts
  2. +1,100 −1,162 packages/upgrade/test/dynamic/upgrade_spec.ts
@@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {Directive, DoCheck, ElementRef, EventEmitter, Inject, Injector, OnChanges, OnInit, SimpleChange, SimpleChanges, Type} from '@angular/core';
import {Directive, DoCheck, ElementRef, EventEmitter, Inject, Injector, OnChanges, OnDestroy, OnInit, SimpleChange, SimpleChanges, Type} from '@angular/core';

import * as angular from '../common/angular1';
import {$SCOPE} from '../common/constants';
@@ -46,23 +46,14 @@ export class UpgradeNg1ComponentAdapterBuilder {
const directive = {selector: selector, inputs: this.inputsRename, outputs: this.outputsRename};

@Directive({jit: true, ...directive})
class MyClass {
// TODO(issue/24571): remove '!'.
directive !: angular.IDirective;
class MyClass extends UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck,
OnDestroy {
constructor(
@Inject($SCOPE) scope: angular.IScope, injector: Injector, elementRef: ElementRef) {
const helper = new UpgradeHelper(injector, name, elementRef, this.directive);
return new UpgradeNg1ComponentAdapter(
helper, scope, self.template, self.inputs, self.outputs, self.propertyOutputs,
self.checkProperties, self.propertyMap) as any;
}
ngOnInit() { /* needs to be here for ng2 to properly detect it */
}
ngOnChanges() { /* needs to be here for ng2 to properly detect it */
}
ngDoCheck() { /* needs to be here for ng2 to properly detect it */
}
ngOnDestroy() { /* needs to be here for ng2 to properly detect it */
super(
new UpgradeHelper(injector, name, elementRef, self.directive || undefined), scope,
self.template, self.inputs, self.outputs, self.propertyOutputs, self.checkProperties,
self.propertyMap) as any;
}
}
this.type = MyClass;
@@ -149,7 +140,7 @@ class UpgradeNg1ComponentAdapter implements OnInit, OnChanges, DoCheck {
private controllerInstance: IControllerInstance|null = null;
destinationObj: IBindingDestination|null = null;
checkLastValues: any[] = [];
private directive: angular.IDirective;
directive: angular.IDirective;
element: Element;
$element: any = null;
componentScope: angular.IScope;
Oops, something went wrong.

0 comments on commit c986d3d

Please sign in to comment.
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.