honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(core): handle synthetic props in Directive host bindings correctly (#35568) 

Prior to this change, animations-related runtime logic assumed that the @HostBinding and @HostListener with synthetic (animations) props are used for Components only. However having @HostBinding and @HostListener with synthetic props on Directives is also supported by View Engine. This commit updates the logic to select correct renderer to execute instructions (current renderer for Directives and sub-component renderer for Components).

This PR resolves #35501.

PR Close #35568
This commit is contained in:
Andrew Kushnir 2020-02-18 17:49:37 -08:00
parent 00e6cb1d62
commit f27deea003
9 changed files with 377 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
packages/core/src/render3/instructions/host_property.ts
View File

@ -8,7 +8,7 @@
import {bindingUpdated} from '../bindings';
import {SanitizerFn} from '../interfaces/sanitization';
import {RENDERER} from '../interfaces/view';
import {getLView, getSelectedTNode, getTView, nextBindingIndex} from '../state';
import {getCurrentDirectiveDef, getLView, getSelectedTNode, getTView, nextBindingIndex} from '../state';
import {NO_CHANGE} from '../tokens';
import {elementPropertyInternal, loadComponentRenderer, storePropertyBindingMetadata} from './shared';
@ -42,7 +42,7 @@ export function ɵɵhostProperty<T>(
/**
* Updates a synthetic host binding (e.g. `[@foo]`) on a component.
* Updates a synthetic host binding (e.g. `[@foo]`) on a component or directive.
*
* This instruction is for compatibility purposes and is designed to ensure that a
* synthetic host binding (e.g. `@HostBinding('@foo')`) properly gets rendered in
@ -70,7 +70,8 @@ export function ɵɵupdateSyntheticHostBinding<T>(
if (bindingUpdated(lView, bindingIndex, value)) {
const tView = getTView();
const tNode = getSelectedTNode();
const renderer = loadComponentRenderer(tNode, lView);
const currentDef = getCurrentDirectiveDef(tView.data);
const renderer = loadComponentRenderer(currentDef, tNode, lView);
elementPropertyInternal(tView, tNode, lView, propName, value, renderer, sanitizer, true);
ngDevMode && storePropertyBindingMetadata(tView.data, tNode, propName, bindingIndex);
}

7
packages/core/src/render3/instructions/listener.ts
View File

@ -15,7 +15,7 @@ import {GlobalTargetResolver, isProceduralRenderer, RElement, Renderer3} from '.
import {isDirectiveHost} from '../interfaces/type_checks';
import {CLEANUP, FLAGS, LView, LViewFlags, RENDERER, TView} from '../interfaces/view';
import {assertNodeOfPossibleTypes} from '../node_assert';
import {getLView, getPreviousOrParentTNode, getTView} from '../state';
import {getCurrentDirectiveDef, getLView, getPreviousOrParentTNode, getTView} from '../state';
import {getComponentLViewByIndex, getNativeByTNode, unwrapRNode} from '../util/view_utils';
import {getLCleanup, handleError, loadComponentRenderer, markViewDirty} from './shared';
@ -48,7 +48,7 @@ export function ɵɵlistener(
}
/**
* Registers a synthetic host listener (e.g. `(@foo.start)`) on a component.
* Registers a synthetic host listener (e.g. `(@foo.start)`) on a component or directive.
*
* This instruction is for compatibility purposes and is designed to ensure that a
* synthetic host listener (e.g. `@HostListener('@foo.start')`) properly gets rendered
@ -73,8 +73,9 @@ export function ɵɵcomponentHostSyntheticListener(
eventTargetResolver?: GlobalTargetResolver): typeof ɵɵcomponentHostSyntheticListener {
const tNode = getPreviousOrParentTNode();
const lView = getLView();
const renderer = loadComponentRenderer(tNode, lView);
const tView = getTView();
const currentDef = getCurrentDirectiveDef(tView.data);
const renderer = loadComponentRenderer(currentDef, tNode, lView);
listenerInternal(
tView, lView, renderer, tNode, eventName, listenerFn, useCapture, eventTargetResolver);
return ɵɵcomponentHostSyntheticListener;

26
packages/core/src/render3/instructions/shared.ts
View File

@ -31,7 +31,7 @@ import {isComponentDef, isComponentHost, isContentQueryHost, isLContainer, isRoo
import {CHILD_HEAD, CHILD_TAIL, CLEANUP, CONTEXT, DECLARATION_COMPONENT_VIEW, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, InitPhaseState, INJECTOR, LView, LViewFlags, NEXT, PARENT, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, T_HOST, TData, TVIEW, TView, TViewType} from '../interfaces/view';
import {assertNodeOfPossibleTypes} from '../node_assert';
import {isInlineTemplate, isNodeMatchingSelectorList} from '../node_selector_matcher';
import {enterView, getBindingsEnabled, getCheckNoChangesMode, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
import {enterView, getBindingsEnabled, getCheckNoChangesMode, getCurrentDirectiveIndex, getIsParent, getPreviousOrParentTNode, getSelectedIndex, getTView, leaveView, setBindingIndex, setBindingRootForHostBindings, setCheckNoChangesMode, setCurrentDirectiveIndex, setCurrentQueryIndex, setPreviousOrParentTNode, setSelectedIndex} from '../state';
import {NO_CHANGE} from '../tokens';
import {isAnimationProp, mergeHostAttrs} from '../util/attrs_utils';
import {INTERPOLATION_DELIMITER, renderStringify, stringifyForError} from '../util/misc_utils';
@ -1272,11 +1272,13 @@ function invokeDirectivesHostBindings(tView: TView, lView: LView, tNode: TNode)
const expando = tView.expandoInstructions!;
const firstCreatePass = tView.firstCreatePass;
const elementIndex = tNode.index - HEADER_OFFSET;
const currentDirectiveIndex = getCurrentDirectiveIndex();
try {
setSelectedIndex(elementIndex);
for (let i = start; i < end; i++) {
const def = tView.data[i] as DirectiveDef<any>;
const directive = lView[i];
for (let dirIndex = start; dirIndex < end; dirIndex++) {
const def = tView.data[dirIndex] as DirectiveDef<unknown>;
const directive = lView[dirIndex];
setCurrentDirectiveIndex(dirIndex);
if (def.hostBindings !== null || def.hostVars !== 0 || def.hostAttrs !== null) {
invokeHostBindingsInCreationMode(def, directive);
} else if (firstCreatePass) {
@ -1285,6 +1287,7 @@ function invokeDirectivesHostBindings(tView: TView, lView: LView, tNode: TNode)
}
} finally {
setSelectedIndex(-1);
setCurrentDirectiveIndex(currentDirectiveIndex);
}
}
@ -1942,9 +1945,18 @@ function getTViewCleanup(tView: TView): any[] {
* There are cases where the sub component's renderer needs to be included
* instead of the current renderer (see the componentSyntheticHost* instructions).
*/
export function loadComponentRenderer(tNode: TNode, lView: LView): Renderer3 {
const componentLView = unwrapLView(lView[tNode.index])!;
return componentLView[RENDERER];
export function loadComponentRenderer(
currentDef: DirectiveDef<any>|null, tNode: TNode, lView: LView): Renderer3 {
// TODO(FW-2043): the `currentDef` is null when host bindings are invoked while creating root
// component (see packages/core/src/render3/component.ts). This is not consistent with the process
// of creating inner components, when current directive index is available in the state. In order
// to avoid relying on current def being `null` (thus special-casing root component creation), the
// process of creating root component should be unified with the process of creating inner
// components.
if (currentDef === null || isComponentDef(currentDef)) {
lView = unwrapLView(lView[tNode.index])!;
}
return lView[RENDERER];
}
/** Handles an error thrown in an LView. */

15
packages/core/src/render3/instructions/styling.ts
View File

@ -22,7 +22,7 @@ import {SanitizerFn} from '../interfaces/sanitization';
import {getTStylingRangeNext, getTStylingRangeNextDuplicate, getTStylingRangePrev, getTStylingRangePrevDuplicate, TStylingKey, TStylingRange} from '../interfaces/styling';
import {HEADER_OFFSET, LView, RENDERER, TData, TView} from '../interfaces/view';
import {applyStyling} from '../node_manipulation';
import {getCurrentDirectiveIndex, getCurrentStyleSanitizer, getLView, getSelectedIndex, getTView, incrementBindingIndex, setCurrentStyleSanitizer} from '../state';
import {getCurrentDirectiveDef, getCurrentStyleSanitizer, getLView, getSelectedIndex, getTView, incrementBindingIndex, setCurrentStyleSanitizer} from '../state';
import {insertTStylingBinding} from '../styling/style_binding_list';
import {getLastParsedKey, getLastParsedValue, parseClassName, parseClassNameNext, parseStyle, parseStyleNext} from '../styling/styling_parser';
import {NO_CHANGE} from '../tokens';
@ -337,7 +337,7 @@ function stylingFirstUpdatePass(
*/
export function wrapInStaticStylingKey(
tData: TData, tNode: TNode, stylingKey: TStylingKey, isClassBased: boolean): TStylingKey {
const hostDirectiveDef = getHostDirectiveDef(tData);
const hostDirectiveDef = getCurrentDirectiveDef(tData);
let residual = isClassBased ? tNode.residualClasses : tNode.residualStyles;
if (hostDirectiveDef === null) {
// We are in template node.
@ -583,17 +583,6 @@ function collectStylingFromTAttrs(
return stylingKey === undefined ? null : stylingKey;
}
/**
* Retrieve the current `DirectiveDef` which is active when `hostBindings` style instruction is
* being executed (or `null` if we are in `template`.)
*
* @param tData Current `TData` where the `DirectiveDef` will be looked up at.
*/
export function getHostDirectiveDef(tData: TData): DirectiveDef<any>|null {
const currentDirectiveIndex = getCurrentDirectiveIndex();
return currentDirectiveIndex === -1 ? null : tData[currentDirectiveIndex] as DirectiveDef<any>;
}
/**
* Convert user input to `KeyValueArray`.
*

25
packages/core/src/render3/state.ts
View File

@ -9,8 +9,9 @@
import {StyleSanitizeFn} from '../sanitization/style_sanitizer';
import {assertDefined, assertEqual} from '../util/assert';
import {assertLViewOrUndefined} from './assert';
import {DirectiveDef} from './interfaces/definition';
import {TNode} from './interfaces/node';
import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState, TVIEW, TView} from './interfaces/view';
import {CONTEXT, DECLARATION_VIEW, LView, OpaqueViewState, TData, TVIEW, TView} from './interfaces/view';
import {MATH_ML_NAMESPACE, SVG_NAMESPACE} from './namespaces';
import {getTNode} from './util/view_utils';
@ -344,7 +345,7 @@ export function setBindingRootForHostBindings(
bindingRootIndex: number, currentDirectiveIndex: number) {
const lFrame = instructionState.lFrame;
lFrame.bindingIndex = lFrame.bindingRootIndex = bindingRootIndex;
lFrame.currentDirectiveIndex = currentDirectiveIndex;
setCurrentDirectiveIndex(currentDirectiveIndex);
}
/**
@ -356,6 +357,26 @@ export function getCurrentDirectiveIndex(): number {
return instructionState.lFrame.currentDirectiveIndex;
}
/**
* Sets an index of a directive whose `hostBindings` are being processed.
*
* @param currentDirectiveIndex `TData` index where current directive instance can be found.
*/
export function setCurrentDirectiveIndex(currentDirectiveIndex: number): void {
instructionState.lFrame.currentDirectiveIndex = currentDirectiveIndex;
}
/**
* Retrieve the current `DirectiveDef` which is active when `hostBindings` instruction is being
* executed.
*
* @param tData Current `TData` where the `DirectiveDef` will be looked up at.
*/
export function getCurrentDirectiveDef(tData: TData): DirectiveDef<any>|null {
const currentDirectiveIndex = instructionState.lFrame.currentDirectiveIndex;
return currentDirectiveIndex === -1 ? null : tData[currentDirectiveIndex] as DirectiveDef<any>;
}
export function getCurrentQueryIndex(): number {
return instructionState.lFrame.currentQueryIndex;
}

310
packages/core/test/acceptance/host_binding_spec.ts
View File

@ -6,10 +6,13 @@
* found in the LICENSE file at https://angular.io/license
*/
import {state, style, transition, trigger} from '@angular/animations';
import {CommonModule} from '@angular/common';
import {AfterContentInit, Component, ComponentFactoryResolver, ComponentRef, ContentChildren, Directive, DoCheck, HostBinding, HostListener, Injectable, Input, NgModule, OnChanges, OnInit, QueryList, ViewChild, ViewChildren, ViewContainerRef} from '@angular/core';
import {bypassSanitizationTrustHtml, bypassSanitizationTrustStyle, bypassSanitizationTrustUrl} from '@angular/core/src/sanitization/bypass';
import {TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {NoopAnimationsModule} from '@angular/platform-browser/animations';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
describe('host bindings', () => {
@ -175,6 +178,313 @@ describe('host bindings', () => {
});
});
describe('with synthetic (animations) props', () => {
it('should work when directive contains synthetic props', () => {
@Directive({
selector: '[animationPropDir]',
})
class AnimationPropDir {
@HostBinding('@myAnimation') myAnimation: string = 'color';
}
@Component({
selector: 'my-comp',
template: '<div animationPropDir>Some content</div>',
animations: [
trigger('myAnimation', [state('color', style({color: 'red'}))]),
],
})
class Comp {
}
TestBed.configureTestingModule({
declarations: [Comp, AnimationPropDir],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
const queryResult = fixture.debugElement.query(By.directive(AnimationPropDir));
expect(queryResult.nativeElement.style.color).toBe('red');
});
it('should work when directive contains synthetic props and directive is applied to a component',
() => {
@Directive({
selector: '[animationPropDir]',
})
class AnimationPropDir {
@HostBinding('@myAnimation') myAnimation: string = 'color';
}
@Component({
selector: 'my-comp',
template: 'Some content',
animations: [
trigger('myAnimation', [state('color', style({color: 'red'}))]),
],
})
class Comp {
}
@Component({
selector: 'app',
template: '<my-comp animationPropDir></my-comp>',
animations: [
trigger('myAnimation', [state('color', style({color: 'green'}))]),
],
})
class App {
}
TestBed.configureTestingModule({
declarations: [App, Comp, AnimationPropDir],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
const queryResult = fixture.debugElement.query(By.directive(AnimationPropDir));
expect(queryResult.nativeElement.style.color).toBe('green');
});
it('should work when component contains synthetic props', () => {
@Component({
selector: 'my-comp',
template: '<div>Some content/div>',
animations: [
trigger('myAnimation', [state('color', style({color: 'red'}))]),
],
})
class Comp {
@HostBinding('@myAnimation') myAnimation: string = 'color';
}
TestBed.configureTestingModule({
declarations: [Comp],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
expect(fixture.nativeElement.style.color).toBe('red');
});
it('should work when child component contains synthetic props', () => {
@Component({
selector: 'my-comp',
template: '<div>Some content/div>',
animations: [
trigger('myAnimation', [state('color', style({color: 'red'}))]),
],
})
class Comp {
@HostBinding('@myAnimation') myAnimation: string = 'color';
}
@Component({
template: '<my-comp></my-comp>',
})
class App {
}
TestBed.configureTestingModule({
declarations: [App, Comp],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
const queryResult = fixture.debugElement.query(By.directive(Comp));
expect(queryResult.nativeElement.style.color).toBe('red');
});
it('should work when component extends a directive that contains synthetic props', () => {
@Directive({
selector: 'animation-dir',
})
class AnimationDir {
@HostBinding('@myAnimation') myAnimation: string = 'color';
}
@Component({
selector: 'my-comp',
template: '<div>Some content</div>',
animations: [
trigger('myAnimation', [state('color', style({color: 'red'}))]),
],
})
class Comp extends AnimationDir {
}
TestBed.configureTestingModule({
declarations: [Comp, AnimationDir],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
expect(fixture.nativeElement.style.color).toBe('red');
});
it('should work when directive contains synthetic listeners', async () => {
const events: string[] = [];
@Directive({
selector: '[animationPropDir]',
})
class AnimationPropDir {
@HostBinding('@myAnimation') myAnimation: string = 'a';
@HostListener('@myAnimation.start')
onAnimationStart() {
events.push('@myAnimation.start');
}
@HostListener('@myAnimation.done')
onAnimationDone() {
events.push('@myAnimation.done');
}
}
@Component({
selector: 'my-comp',
template: '<div animationPropDir>Some content</div>',
animations: [
trigger('myAnimation', [state('a', style({color: 'yellow'})), transition('* => a', [])]),
],
})
class Comp {
}
TestBed.configureTestingModule({
declarations: [Comp, AnimationPropDir],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
await fixture.whenStable(); // wait for animations to complete
const queryResult = fixture.debugElement.query(By.directive(AnimationPropDir));
expect(queryResult.nativeElement.style.color).toBe('yellow');
expect(events).toEqual(['@myAnimation.start', '@myAnimation.done']);
});
it('should work when component contains synthetic listeners', async () => {
const events: string[] = [];
@Component({
selector: 'my-comp',
template: '<div>Some content</div>',
animations: [
trigger('myAnimation', [state('a', style({color: 'yellow'})), transition('* => a', [])]),
],
})
class Comp {
@HostBinding('@myAnimation') myAnimation: string = 'a';
@HostListener('@myAnimation.start')
onAnimationStart() {
events.push('@myAnimation.start');
}
@HostListener('@myAnimation.done')
onAnimationDone() {
events.push('@myAnimation.done');
}
}
TestBed.configureTestingModule({
declarations: [Comp],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
await fixture.whenStable(); // wait for animations to complete
expect(fixture.nativeElement.style.color).toBe('yellow');
expect(events).toEqual(['@myAnimation.start', '@myAnimation.done']);
});
it('should work when child component contains synthetic listeners', async () => {
const events: string[] = [];
@Component({
selector: 'my-comp',
template: '<div>Some content</div>',
animations: [
trigger('myAnimation', [state('a', style({color: 'yellow'})), transition('* => a', [])]),
],
})
class Comp {
@HostBinding('@myAnimation') myAnimation: string = 'a';
@HostListener('@myAnimation.start')
onAnimationStart() {
events.push('@myAnimation.start');
}
@HostListener('@myAnimation.done')
onAnimationDone() {
events.push('@myAnimation.done');
}
}
@Component({
template: '<my-comp></my-comp>',
})
class App {
}
TestBed.configureTestingModule({
declarations: [App, Comp],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(App);
fixture.detectChanges();
await fixture.whenStable(); // wait for animations to complete
const queryResult = fixture.debugElement.query(By.directive(Comp));
expect(queryResult.nativeElement.style.color).toBe('yellow');
expect(events).toEqual(['@myAnimation.start', '@myAnimation.done']);
});
it('should work when component extends a directive that contains synthetic listeners',
async () => {
const events: string[] = [];
@Directive({
selector: 'animation-dir',
})
class AnimationDir {
@HostBinding('@myAnimation') myAnimation: string = 'a';
@HostListener('@myAnimation.start')
onAnimationStart() {
events.push('@myAnimation.start');
}
@HostListener('@myAnimation.done')
onAnimationDone() {
events.push('@myAnimation.done');
}
}
@Component({
selector: 'my-comp',
template: '<div>Some content</div>',
animations: [
trigger(
'myAnimation', [state('a', style({color: 'yellow'})), transition('* => a', [])]),
],
})
class Comp extends AnimationDir {
}
TestBed.configureTestingModule({
declarations: [Comp],
imports: [NoopAnimationsModule],
});
const fixture = TestBed.createComponent(Comp);
fixture.detectChanges();
await fixture.whenStable(); // wait for animations to complete
expect(fixture.nativeElement.style.color).toBe('yellow');
expect(events).toEqual(['@myAnimation.start', '@myAnimation.done']);
});
});
describe('via @HostBinding', () => {
it('should render styling for parent and sub-classed components in order', () => {
@Component({

6
packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json
View File

@ -311,6 +311,9 @@
{
"name": "getContainerRenderParent"
},
{
"name": "getCurrentDirectiveIndex"
},
{
"name": "getDirectiveDef"
},
@ -587,6 +590,9 @@
{
"name": "setBindingRootForHostBindings"
},
{
"name": "setCurrentDirectiveIndex"
},
{
"name": "setCurrentQueryIndex"
},

3
packages/core/test/bundling/hello_world/bundle.golden_symbols.json
View File

@ -455,6 +455,9 @@
{
"name": "setBindingRootForHostBindings"
},
{
"name": "setCurrentDirectiveIndex"
},
{
"name": "setCurrentQueryIndex"
},

9
packages/core/test/bundling/todo/bundle.golden_symbols.json
View File

@ -587,6 +587,9 @@
{
"name": "getContextLView"
},
{
"name": "getCurrentDirectiveDef"
},
{
"name": "getCurrentDirectiveIndex"
},
@ -614,9 +617,6 @@
{
"name": "getFirstNativeNode"
},
{
"name": "getHostDirectiveDef"
},
{
"name": "getInjectableDef"
},
@ -1100,6 +1100,9 @@
{
"name": "setCheckNoChangesMode"
},
{
"name": "setCurrentDirectiveIndex"
},
{
"name": "setCurrentQueryIndex"
},