fix(ivy): fix property names of ngOnChanges (#27714)

- #reslove FW-812
- #reslove FW-844

PR Close #27714
This commit is contained in:
Misko Hevery 2018-12-17 13:17:42 -08:00 committed by Miško Hevery
parent 4774a1abff
commit 1c93afe956
8 changed files with 237 additions and 217 deletions

View File

@ -110,7 +110,7 @@ function baseDirectiveFields(
meta, elVarExp, contextVarExp, styleBuilder, bindingParser, constantPool, hostVarsCount));
// e.g 'inputs: {a: 'a'}`
definitionMap.set('inputs', conditionallyCreateMapObjectLiteral(meta.inputs));
definitionMap.set('inputs', conditionallyCreateMapObjectLiteral(meta.inputs, true));
// e.g 'outputs: {a: 'a'}`
definitionMap.set('outputs', conditionallyCreateMapObjectLiteral(meta.outputs));

View File

@ -67,20 +67,36 @@ export function asLiteral(value: any): o.Expression {
return o.literal(value, o.INFERRED_TYPE);
}
export function conditionallyCreateMapObjectLiteral(keys: {[key: string]: string | string[]}):
o.Expression|null {
export function conditionallyCreateMapObjectLiteral(
keys: {[key: string]: string | string[]}, keepDeclared?: boolean): o.Expression|null {
if (Object.getOwnPropertyNames(keys).length > 0) {
return mapToExpression(keys);
return mapToExpression(keys, keepDeclared);
}
return null;
}
function mapToExpression(map: {[key: string]: any}): o.Expression {
function mapToExpression(
map: {[key: string]: string | string[]}, keepDeclared?: boolean): o.Expression {
return o.literalMap(Object.getOwnPropertyNames(map).map(key => {
// canonical syntax: `dirProp: elProp`
// canonical syntax: `dirProp: publicProp`
// if there is no `:`, use dirProp = elProp
const parts = splitAtColon(key, [key, map[key]]);
return {key: parts[0], quoted: false, value: asLiteral(parts[1])};
const value = map[key];
let declaredName: string;
let publicName: string;
let minifiedName: string;
if (Array.isArray(value)) {
[publicName, declaredName] = value;
} else {
[declaredName, publicName] = splitAtColon(key, [key, value]);
}
minifiedName = declaredName;
return {
key: minifiedName,
quoted: false,
value: (keepDeclared && publicName !== declaredName) ?
o.literalArr([asLiteral(publicName), asLiteral(declaredName)]) :
asLiteral(publicName)
};
}));
}

View File

@ -345,8 +345,8 @@ export function defineNgModule<T>(def: {type: T} & Partial<NgModuleDef<T>>): nev
* @Input()
* propName1: string;
*
* @Input('publicName')
* propName2: number;
* @Input('publicName2')
* declaredPropName2: number;
* }
* ```
*
@ -354,26 +354,35 @@ export function defineNgModule<T>(def: {type: T} & Partial<NgModuleDef<T>>): nev
*
* ```
* {
* a0: 'propName1',
* b1: ['publicName', 'propName2'],
* propName1: 'propName1',
* declaredPropName2: ['publicName2', 'declaredPropName2'],
* }
* ```
*
* becomes
* which is than translated by the minifier as:
*
* ```
* {
* 'propName1': 'a0',
* 'publicName': 'b1'
* minifiedPropName1: 'propName1',
* minifiedPropName2: ['publicName2', 'declaredPropName2'],
* }
* ```
*
* Optionally the function can take `secondary` which will result in:
* becomes: (public name => minifiedName)
*
* ```
* {
* 'propName1': 'a0',
* 'propName2': 'b1'
* 'propName1': 'minifiedPropName1',
* 'publicName2': 'minifiedPropName2',
* }
* ```
*
* Optionally the function can take `secondary` which will result in: (public name => declared name)
*
* ```
* {
* 'propName1': 'propName1',
* 'publicName2': 'declaredPropName2',
* }
* ```
*
@ -384,7 +393,7 @@ function invertObject(obj: any, secondary?: any): any {
const newLookup: any = {};
for (const minifiedKey in obj) {
if (obj.hasOwnProperty(minifiedKey)) {
let publicName = obj[minifiedKey];
let publicName: string = obj[minifiedKey];
let declaredName = publicName;
if (Array.isArray(publicName)) {
declaredName = publicName[1];
@ -392,7 +401,7 @@ function invertObject(obj: any, secondary?: any): any {
}
newLookup[publicName] = minifiedKey;
if (secondary) {
(secondary[declaredName] = minifiedKey);
(secondary[publicName] = declaredName);
}
}
}

View File

@ -39,11 +39,13 @@ type OnChangesExpando = OnChanges & {
* ```
*/
export function NgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
const declaredToMinifiedInputs = definition.declaredInputs;
const publicToDeclaredInputs = definition.declaredInputs;
const publicToMinifiedInputs = definition.inputs;
const proto = definition.type.prototype;
for (const declaredName in declaredToMinifiedInputs) {
if (declaredToMinifiedInputs.hasOwnProperty(declaredName)) {
const minifiedKey = declaredToMinifiedInputs[declaredName];
for (const publicName in publicToDeclaredInputs) {
if (publicToDeclaredInputs.hasOwnProperty(publicName)) {
const minifiedKey = publicToMinifiedInputs[publicName];
const declaredKey = publicToDeclaredInputs[publicName];
const privateMinKey = PRIVATE_PREFIX + minifiedKey;
// Walk the prototype chain to see if we find a property descriptor
@ -72,12 +74,12 @@ export function NgOnChangesFeature<T>(definition: DirectiveDef<T>): void {
}
const isFirstChange = !this.hasOwnProperty(privateMinKey);
const currentChange = simpleChanges[declaredName];
const currentChange = simpleChanges[declaredKey];
if (currentChange) {
currentChange.currentValue = value;
} else {
simpleChanges[declaredName] =
simpleChanges[declaredKey] =
new SimpleChange(this[privateMinKey], value, isFirstChange);
}

View File

@ -84,10 +84,10 @@ describe('InheritDefinitionFeature', () => {
qux: 'subQux',
});
expect(subDef.declaredInputs).toEqual({
declaredFoo: 'superFoo',
bar: 'superBar',
baz: 'subBaz',
qux: 'subQux',
foo: 'declaredFoo',
bar: 'bar',
baz: 'baz',
qux: 'qux',
});
});
@ -228,7 +228,7 @@ describe('InheritDefinitionFeature', () => {
expect(subDef.declaredInputs).toEqual({
input1: 'input1',
input2: 'input2',
input3: 'input3',
alias3: 'input3',
input4: 'input4',
input5: 'input5',
});

View File

@ -264,7 +264,7 @@ ivyEnabled && describe('render3 jit', () => {
const InputCompAny = InputComp as any;
expect(InputCompAny.ngComponentDef.inputs).toEqual({publicName: 'privateName'});
expect(InputCompAny.ngComponentDef.declaredInputs).toEqual({privateName: 'privateName'});
expect(InputCompAny.ngComponentDef.declaredInputs).toEqual({publicName: 'privateName'});
});
it('should add @Input properties to a directive', () => {
@ -277,7 +277,7 @@ ivyEnabled && describe('render3 jit', () => {
const InputDirAny = InputDir as any;
expect(InputDirAny.ngDirectiveDef.inputs).toEqual({publicName: 'privateName'});
expect(InputDirAny.ngDirectiveDef.declaredInputs).toEqual({privateName: 'privateName'});
expect(InputDirAny.ngDirectiveDef.declaredInputs).toEqual({publicName: 'privateName'});
});
it('should add ngBaseDef to types with @Input properties', () => {

View File

@ -1915,194 +1915,189 @@ withEachNg1Version(() => {
});
}));
fixmeIvy(
'FW-844: Directive input bindings cannot be assigned after the `@Directive` decorator has been compiled')
.it('should call `$onChanges()` on binding destination', fakeAsync(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const $onChangesControllerSpyA = jasmine.createSpy('$onChangesControllerA');
const $onChangesControllerSpyB = jasmine.createSpy('$onChangesControllerB');
const $onChangesScopeSpy = jasmine.createSpy('$onChangesScope');
let ng2Instance: any;
it('should call `$onChanges()` on binding destination', fakeAsync(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const $onChangesControllerSpyA = jasmine.createSpy('$onChangesControllerA');
const $onChangesControllerSpyB = jasmine.createSpy('$onChangesControllerB');
const $onChangesScopeSpy = jasmine.createSpy('$onChangesScope');
let ng2Instance: any;
@Component({
selector: 'ng2',
template: '<ng1-a [valA]="val"></ng1-a> | <ng1-b [valB]="val"></ng1-b>'
})
class Ng2Component {
constructor() { ng2Instance = this; }
}
@Component({
selector: 'ng2',
template: '<ng1-a [valA]="val"></ng1-a> | <ng1-b [valB]="val"></ng1-b>'
})
class Ng2Component {
constructor() { ng2Instance = this; }
}
angular.module('ng1', [])
.directive('ng1A', () => ({
template: '',
scope: {valA: '<'},
bindToController: true,
controllerAs: '$ctrl',
controller: function($scope: angular.IScope) {
this.$onChanges = $onChangesControllerSpyA;
}
}))
.directive('ng1B', () => ({
template: '',
scope: {valB: '<'},
bindToController: false,
controllerAs: '$ctrl',
controller: class {
$onChanges(changes: SimpleChanges) {
$onChangesControllerSpyB(changes);
}
}
}))
.directive('ng2', adapter.downgradeNg2Component(Ng2Component))
.run(($rootScope: angular.IRootScopeService) => {
Object.getPrototypeOf($rootScope).$onChanges = $onChangesScopeSpy;
});
angular.module('ng1', [])
.directive('ng1A', () => ({
template: '',
scope: {valA: '<'},
bindToController: true,
controllerAs: '$ctrl',
controller: function($scope: angular.IScope) {
this.$onChanges = $onChangesControllerSpyA;
}
}))
.directive(
'ng1B',
() => ({
template: '',
scope: {valB: '<'},
bindToController: false,
controllerAs: '$ctrl',
controller: class {
$onChanges(changes: SimpleChanges) { $onChangesControllerSpyB(changes); }
}
}))
.directive('ng2', adapter.downgradeNg2Component(Ng2Component))
.run(($rootScope: angular.IRootScopeService) => {
Object.getPrototypeOf($rootScope).$onChanges = $onChangesScopeSpy;
});
@NgModule({
declarations: [
adapter.upgradeNg1Component('ng1A'), adapter.upgradeNg1Component('ng1B'),
Ng2Component
],
imports: [BrowserModule],
})
class Ng2Module {
}
@NgModule({
declarations: [
adapter.upgradeNg1Component('ng1A'), adapter.upgradeNg1Component('ng1B'),
Ng2Component
],
imports: [BrowserModule],
})
class Ng2Module {
}
const element = html(`<div><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
// Initial `$onChanges()` call
tick();
const element = html(`<div><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
// Initial `$onChanges()` call
tick();
expect($onChangesControllerSpyA.calls.count()).toBe(1);
expect($onChangesControllerSpyA.calls.argsFor(0)[0]).toEqual({
valA: jasmine.any(SimpleChange)
});
expect($onChangesControllerSpyA.calls.count()).toBe(1);
expect($onChangesControllerSpyA.calls.argsFor(0)[0]).toEqual({
valA: jasmine.any(SimpleChange)
});
expect($onChangesControllerSpyB).not.toHaveBeenCalled();
expect($onChangesControllerSpyB).not.toHaveBeenCalled();
expect($onChangesScopeSpy.calls.count()).toBe(1);
expect($onChangesScopeSpy.calls.argsFor(0)[0]).toEqual({
valB: jasmine.any(SimpleChange)
});
expect($onChangesScopeSpy.calls.count()).toBe(1);
expect($onChangesScopeSpy.calls.argsFor(0)[0]).toEqual({
valB: jasmine.any(SimpleChange)
});
$onChangesControllerSpyA.calls.reset();
$onChangesControllerSpyB.calls.reset();
$onChangesScopeSpy.calls.reset();
$onChangesControllerSpyA.calls.reset();
$onChangesControllerSpyB.calls.reset();
$onChangesScopeSpy.calls.reset();
// `$onChanges()` call after a change
ng2Instance.val = 'new value';
tick();
ref.ng1RootScope.$digest();
// `$onChanges()` call after a change
ng2Instance.val = 'new value';
tick();
ref.ng1RootScope.$digest();
expect($onChangesControllerSpyA.calls.count()).toBe(1);
expect($onChangesControllerSpyA.calls.argsFor(0)[0]).toEqual({
valA: jasmine.objectContaining({currentValue: 'new value'})
});
expect($onChangesControllerSpyA.calls.count()).toBe(1);
expect($onChangesControllerSpyA.calls.argsFor(0)[0]).toEqual({
valA: jasmine.objectContaining({currentValue: 'new value'})
});
expect($onChangesControllerSpyB).not.toHaveBeenCalled();
expect($onChangesControllerSpyB).not.toHaveBeenCalled();
expect($onChangesScopeSpy.calls.count()).toBe(1);
expect($onChangesScopeSpy.calls.argsFor(0)[0]).toEqual({
valB: jasmine.objectContaining({currentValue: 'new value'})
});
expect($onChangesScopeSpy.calls.count()).toBe(1);
expect($onChangesScopeSpy.calls.argsFor(0)[0]).toEqual({
valB: jasmine.objectContaining({currentValue: 'new value'})
});
ref.dispose();
});
}));
ref.dispose();
});
}));
fixmeIvy(
'FW-843: destroy hooks are not registered on upgraded ng1 components contained in ng2 component templates under ivy')
.it('should call `$onDestroy()` on controller', fakeAsync(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const $onDestroySpyA = jasmine.createSpy('$onDestroyA');
const $onDestroySpyB = jasmine.createSpy('$onDestroyB');
let ng2ComponentInstance: Ng2Component;
it('should call `$onDestroy()` on controller', fakeAsync(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const $onDestroySpyA = jasmine.createSpy('$onDestroyA');
const $onDestroySpyB = jasmine.createSpy('$onDestroyB');
let ng2ComponentInstance: Ng2Component;
@Component({
selector: 'ng2',
template: `
@Component({
selector: 'ng2',
template: `
<div *ngIf="!ng2Destroy">
<ng1-a></ng1-a> | <ng1-b></ng1-b>
</div>
`
})
class Ng2Component {
ng2Destroy: boolean = false;
constructor() { ng2ComponentInstance = this; }
}
})
class Ng2Component {
ng2Destroy: boolean = false;
constructor() { ng2ComponentInstance = this; }
}
// On browsers that don't support `requestAnimationFrame` (IE 9, Android <= 4.3),
// `$animate` will use `setTimeout(..., 16.6)` instead. This timeout will still be
// on
// the queue at the end of the test, causing it to fail.
// Mocking animations (via `ngAnimateMock`) avoids the issue.
angular.module('ng1', ['ngAnimateMock'])
.directive('ng1A', () => ({
template: '',
scope: {},
bindToController: true,
controllerAs: '$ctrl',
controller: class {$onDestroy() { $onDestroySpyA(); }}
}))
.directive(
'ng1B', () => ({
template: '',
scope: {},
bindToController: false,
controllerAs: '$ctrl',
controller: function() { this.$onDestroy = $onDestroySpyB; }
}))
.directive('ng2', adapter.downgradeNg2Component(Ng2Component));
// On browsers that don't support `requestAnimationFrame` (IE 9, Android <= 4.3),
// `$animate` will use `setTimeout(..., 16.6)` instead. This timeout will still be
// on
// the queue at the end of the test, causing it to fail.
// Mocking animations (via `ngAnimateMock`) avoids the issue.
angular.module('ng1', ['ngAnimateMock'])
.directive('ng1A', () => ({
template: '',
scope: {},
bindToController: true,
controllerAs: '$ctrl',
controller: class {$onDestroy() { $onDestroySpyA(); }}
}))
.directive('ng1B', () => ({
template: '',
scope: {},
bindToController: false,
controllerAs: '$ctrl',
controller: function() { this.$onDestroy = $onDestroySpyB; }
}))
.directive('ng2', adapter.downgradeNg2Component(Ng2Component));
@NgModule({
declarations: [
adapter.upgradeNg1Component('ng1A'), adapter.upgradeNg1Component('ng1B'),
Ng2Component
],
imports: [BrowserModule],
})
class Ng2Module {
}
@NgModule({
declarations: [
adapter.upgradeNg1Component('ng1A'), adapter.upgradeNg1Component('ng1B'),
Ng2Component
],
imports: [BrowserModule],
})
class Ng2Module {
}
const element = html(`<div ng-if="!ng1Destroy"><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
const $rootScope = ref.ng1RootScope as any;
const element = html(`<div ng-if="!ng1Destroy"><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
const $rootScope = ref.ng1RootScope as any;
$rootScope.ng1Destroy = false;
tick();
$rootScope.$digest();
$rootScope.ng1Destroy = false;
tick();
$rootScope.$digest();
expect($onDestroySpyA).not.toHaveBeenCalled();
expect($onDestroySpyB).not.toHaveBeenCalled();
expect($onDestroySpyA).not.toHaveBeenCalled();
expect($onDestroySpyB).not.toHaveBeenCalled();
$rootScope.ng1Destroy = true;
tick();
$rootScope.$digest();
$rootScope.ng1Destroy = true;
tick();
$rootScope.$digest();
expect($onDestroySpyA).toHaveBeenCalled();
expect($onDestroySpyB).toHaveBeenCalled();
expect($onDestroySpyA).toHaveBeenCalled();
expect($onDestroySpyB).toHaveBeenCalled();
$onDestroySpyA.calls.reset();
$onDestroySpyB.calls.reset();
$onDestroySpyA.calls.reset();
$onDestroySpyB.calls.reset();
$rootScope.ng1Destroy = false;
tick();
$rootScope.$digest();
$rootScope.ng1Destroy = false;
tick();
$rootScope.$digest();
expect($onDestroySpyA).not.toHaveBeenCalled();
expect($onDestroySpyB).not.toHaveBeenCalled();
expect($onDestroySpyA).not.toHaveBeenCalled();
expect($onDestroySpyB).not.toHaveBeenCalled();
ng2ComponentInstance.ng2Destroy = true;
tick();
$rootScope.$digest();
ng2ComponentInstance.ng2Destroy = true;
tick();
$rootScope.$digest();
expect($onDestroySpyA).toHaveBeenCalled();
expect($onDestroySpyB).toHaveBeenCalled();
expect($onDestroySpyA).toHaveBeenCalled();
expect($onDestroySpyB).toHaveBeenCalled();
ref.dispose();
});
}));
ref.dispose();
});
}));
it('should not call `$onDestroy()` on scope', fakeAsync(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
@ -3053,40 +3048,37 @@ withEachNg1Version(() => {
}));
});
fixmeIvy(
'FW-844: Directive input bindings cannot be assigned after the `@Directive` decorator has been compiled')
.it('should bind input properties (<) of components', async(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const ng1Module = angular.module('ng1', []);
it('should bind input properties (<) of components', async(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));
const ng1Module = angular.module('ng1', []);
const ng1 = {
bindings: {personProfile: '<'},
template:
'Hello {{$ctrl.personProfile.firstName}} {{$ctrl.personProfile.lastName}}',
controller: class {}
};
ng1Module.component('ng1', ng1);
const ng1 = {
bindings: {personProfile: '<'},
template: 'Hello {{$ctrl.personProfile.firstName}} {{$ctrl.personProfile.lastName}}',
controller: class {}
};
ng1Module.component('ng1', ng1);
@Component({selector: 'ng2', template: '<ng1 [personProfile]="goku"></ng1>'})
class Ng2 {
goku = {firstName: 'GOKU', lastName: 'SAN'};
}
@Component({selector: 'ng2', template: '<ng1 [personProfile]="goku"></ng1>'})
class Ng2 {
goku = {firstName: 'GOKU', lastName: 'SAN'};
}
@NgModule({
declarations: [adapter.upgradeNg1Component('ng1'), Ng2],
imports: [BrowserModule],
})
class Ng2Module {
}
@NgModule({
declarations: [adapter.upgradeNg1Component('ng1'), Ng2],
imports: [BrowserModule],
})
class Ng2Module {
}
ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2));
ng1Module.directive('ng2', adapter.downgradeNg2Component(Ng2));
const element = html(`<div><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
expect(multiTrim(document.body.textContent)).toEqual(`Hello GOKU SAN`);
ref.dispose();
});
}));
const element = html(`<div><ng2></ng2></div>`);
adapter.bootstrap(element, ['ng1']).ready((ref) => {
expect(multiTrim(document.body.textContent)).toEqual(`Hello GOKU SAN`);
ref.dispose();
});
}));
it('should support ng2 > ng1 > ng2', async(() => {
const adapter: UpgradeAdapter = new UpgradeAdapter(forwardRef(() => Ng2Module));

View File

@ -3968,6 +3968,7 @@ withEachNg1Version(() => {
});
}));
// fixmeIvy('FW-724: upgraded ng1 components are not being rendered')
it('should support ng2 > ng1 > ng2 (with inputs/outputs)', fakeAsync(() => {
let ng2ComponentAInstance: Ng2ComponentA;
let ng2ComponentBInstance: Ng2ComponentB;