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

fix(core): Host classes should not be fed back into `@Input` (#35889) 

Previously all static styling information (including the ones from component/directive host bindings) would get merged into a single value before it would be written into the `@Input('class'/'style')`. The new behavior specifically excludes host values from the `@Input` bindings.

Fix #35383

PR Close #35889
This commit is contained in:
Miško Hevery 2020-03-05 16:17:26 -08:00 committed by Kara Erickson
parent aaa89bb715
commit cda2530df5
10 changed files with 154 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/core/src/render3/component.ts
View File

@ -177,7 +177,7 @@ export function createRootComponentView(
const tNode: TElementNode = getOrCreateTNode(tView, null, 0, TNodeType.Element, null, null);
const mergedAttrs = tNode.mergedAttrs = def.hostAttrs;
if (mergedAttrs !== null) {
computeStaticStyling(tNode, mergedAttrs);
computeStaticStyling(tNode, mergedAttrs, true);
if (rNode !== null) {
setUpAttributes(hostRenderer, rNode, mergedAttrs);
if (tNode.classes !== null) {

14
packages/core/src/render3/instructions/element.ts
View File

@ -39,8 +39,12 @@ function elementStartFirstCreatePass(
resolveDirectives(tView, lView, tNode, getConstant<string[]>(tViewConsts, localRefsIndex));
ngDevMode && logUnknownElementError(tView, lView, native, tNode, hasDirectives);
if (tNode.attrs !== null) {
computeStaticStyling(tNode, tNode.attrs, false);
}
if (tNode.mergedAttrs !== null) {
computeStaticStyling(tNode, tNode.mergedAttrs);
computeStaticStyling(tNode, tNode.mergedAttrs, true);
}
if (tView.queries !== null) {
@ -148,12 +152,12 @@ export function ɵɵelementEnd(): void {
}
}
if (tNode.classes !== null && hasClassInput(tNode)) {
setDirectiveInputsWhichShadowsStyling(tView, tNode, getLView(), tNode.classes, true);
if (tNode.classesWithoutHost != null && hasClassInput(tNode)) {
setDirectiveInputsWhichShadowsStyling(tView, tNode, getLView(), tNode.classesWithoutHost, true);
}
if (tNode.styles !== null && hasStyleInput(tNode)) {
setDirectiveInputsWhichShadowsStyling(tView, tNode, getLView(), tNode.styles, false);
if (tNode.stylesWithoutHost != null && hasStyleInput(tNode)) {
setDirectiveInputsWhichShadowsStyling(tView, tNode, getLView(), tNode.stylesWithoutHost, false);
}
}

2
packages/core/src/render3/instructions/element_container.ts
View File

@ -33,7 +33,7 @@ function elementContainerStartFirstCreatePass(
// While ng-container doesn't necessarily support styling, we use the style context to identify
// and execute directives on the ng-container.
if (attrs !== null) {
computeStaticStyling(tNode, attrs);
computeStaticStyling(tNode, attrs, true);
}
const localRefs = getConstant<string[]>(tViewConsts, localRefsIndex);

2
packages/core/src/render3/instructions/lview_debug.ts
View File

@ -179,8 +179,10 @@ class TNode implements ITNode {
public parent: TElementNode|TContainerNode|null, //
public projection: number|(ITNode|RNode[])[]|null, //
public styles: string|null, //
public stylesWithoutHost: string|null, //
public residualStyles: KeyValueArray<any>|undefined|null, //
public classes: string|null, //
public classesWithoutHost: string|null, //
public residualClasses: KeyValueArray<any>|undefined|null, //
public classBindings: TStylingRange, //
public styleBindings: TStylingRange, //

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

@ -851,8 +851,10 @@ export function createTNode(
tParent, // parent: TElementNode|TContainerNode|null
null, // projection: number|(ITNode|RNode[])[]|null
null, // styles: string|null
null, // stylesWithoutHost: string|null
undefined, // residualStyles: string|null
null, // classes: string|null
null, // classesWithoutHost: string|null
undefined, // residualClasses: string|null
0 as any, // classBindings: TStylingRange;
0 as any, // styleBindings: TStylingRange;
@ -881,8 +883,10 @@ export function createTNode(
parent: tParent,
projection: null,
styles: null,
stylesWithoutHost: null,
residualStyles: undefined,
classes: null,
classesWithoutHost: null,
residualClasses: undefined,
classBindings: 0 as any,
styleBindings: 0 as any,

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

@ -221,7 +221,7 @@ export function checkStylingMap(
// the binding has removed it. This would confuse `[ngStyle]`/`[ngClass]` to do the wrong
// thing as it would think that the static portion was removed. For this reason we
// concatenate it so that `[ngStyle]`/`[ngClass]` can continue to work on changed.
let staticPrefix = isClassBased ? tNode.classes : tNode.styles;
let staticPrefix = isClassBased ? tNode.classesWithoutHost : tNode.stylesWithoutHost;
ngDevMode && isClassBased === false && staticPrefix !== null &&
assertEqual(
staticPrefix.endsWith(';'), true, 'Expecting static portion to end with \';\'');

37
packages/core/src/render3/interfaces/node.ts
View File

@ -502,14 +502,30 @@ export interface TNode {
projection: (TNode|RNode[])[]|number|null;
/**
* A collection of all style static values for an element.
* A collection of all `style` static values for an element (including from host).
*
* This field will be populated if and when:
*
* - There are one or more initial styles on an element (e.g. `<div style="width:200px">`)
* - There are one or more initial `style`s on an element (e.g. `<div style="width:200px;">`)
* - There are one or more initial `style`s on a directive/component host
* (e.g. `@Directive({host: {style: "width:200px;" } }`)
*/
styles: string|null;
/**
* A collection of all `style` static values for an element excluding host sources.
*
* Populated when there are one or more initial `style`s on an element
* (e.g. `<div style="width:200px;">`)
* Must be stored separately from `tNode.styles` to facilitate setting directive
* inputs that shadow the `style` property. If we used `tNode.styles` as is for shadowed inputs,
* we would feed host styles back into directives as "inputs". If we used `tNode.attrs`, we would
* have to concatenate the attributes on every template pass. Instead, we process once on first
* create pass and store here.
*/
stylesWithoutHost: string|null;
/**
* A `KeyValueArray` version of residual `styles`.
*
@ -540,14 +556,29 @@ export interface TNode {
residualStyles: KeyValueArray<any>|undefined|null;
/**
* A collection of all class static values for an element.
* A collection of all class static values for an element (including from host).
*
* This field will be populated if and when:
*
* - There are one or more initial classes on an element (e.g. `<div class="one two three">`)
* - There are one or more initial classes on an directive/component host
* (e.g. `@Directive({host: {class: "SOME_CLASS" } }`)
*/
classes: string|null;
/**
* A collection of all class static values for an element excluding host sources.
*
* Populated when there are one or more initial classes on an element
* (e.g. `<div class="SOME_CLASS">`)
* Must be stored separately from `tNode.classes` to facilitate setting directive
* inputs that shadow the `class` property. If we used `tNode.classes` as is for shadowed inputs,
* we would feed host classes back into directives as "inputs". If we used `tNode.attrs`, we would
* have to concatenate the attributes on every template pass. Instead, we process once on first
* create pass and store here.
*/
classesWithoutHost: string|null;
/**
* A `KeyValueArray` version of residual `classes`.
*

36
packages/core/src/render3/styling/static_styling.ts
View File

@ -18,25 +18,31 @@ import {getTView} from '../state';
*
* @param tNode The `TNode` into which the styling information should be loaded.
* @param attrs `TAttributes` containing the styling information.
* @param writeToHost Where should the resulting static styles be written?
* - `false` Write to `TNode.stylesWithoutHost` / `TNode.classesWithoutHost`
* - `true` Write to `TNode.styles` / `TNode.classes`
*/
export function computeStaticStyling(tNode: TNode, attrs: TAttributes): void {
export function computeStaticStyling(
tNode: TNode, attrs: TAttributes|null, writeToHost: boolean): void {
ngDevMode &&
assertFirstCreatePass(getTView(), 'Expecting to be called in first template pass only');
let styles: string|null = tNode.styles;
let classes: string|null = tNode.classes;
let styles: string|null = writeToHost ? tNode.styles : null;
let classes: string|null = writeToHost ? tNode.classes : null;
let mode: AttributeMarker|0 = 0;
for (let i = 0; i < attrs.length; i++) {
const value = attrs[i];
if (typeof value === 'number') {
mode = value;
} else if (mode == AttributeMarker.Classes) {
classes = concatStringsWithSpace(classes, value as string);
} else if (mode == AttributeMarker.Styles) {
const style = value as string;
const styleValue = attrs[++i] as string;
styles = concatStringsWithSpace(styles, style + ': ' + styleValue + ';');
if (attrs !== null) {
for (let i = 0; i < attrs.length; i++) {
const value = attrs[i];
if (typeof value === 'number') {
mode = value;
} else if (mode == AttributeMarker.Classes) {
classes = concatStringsWithSpace(classes, value as string);
} else if (mode == AttributeMarker.Styles) {
const style = value as string;
const styleValue = attrs[++i] as string;
styles = concatStringsWithSpace(styles, style + ': ' + styleValue + ';');
}
}
}
styles !== null && (tNode.styles = styles);
classes !== null && (tNode.classes = classes);
writeToHost ? tNode.styles = styles : tNode.stylesWithoutHost = styles;
writeToHost ? tNode.classes = classes : tNode.classesWithoutHost = classes;
}

83
packages/core/test/acceptance/styling_spec.ts
View File

@ -308,7 +308,7 @@ describe('styling', () => {
// VE has weird behavior where it calls the @Input('class') with either `class="static` or
// `[class]="dynamic"` but never both. This is determined at compile time. Due to locality
// we don't know if `[class]` is coming if we see `class` only. So we need to combine the
// static and dynamic parte. This results in slightly different calling sequence, but should
// static and dynamic parts. This results in slightly different calling sequence, but should
// result in the same final DOM.
expect(div1.getAttribute('shadow-class')).toEqual('s1 d1');
@ -316,6 +316,79 @@ describe('styling', () => {
expect(div2.getAttribute('shadow-class')).toEqual('s2 d2');
});
it('should not feed host classes back into shadow input', () => {
@Component({
template: `
<div class="s1" dir-shadows-class-input></div>
<div class="s1" [class]=" 'd1' " dir-shadows-class-input></div>
`
})
class Cmp {
}
@Directive({selector: '[dir-shadows-class-input]', host: {'class': 'DIRECTIVE'}})
class DirectiveShadowsClassInput {
constructor(private elementRef: ElementRef) {}
@Input('class')
set klass(value: string) {
this.elementRef.nativeElement.setAttribute('shadow-class', value);
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsClassInput]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const [divStatic, divBinding] = fixture.nativeElement.querySelectorAll('div');
expectClass(divStatic).toEqual({'DIRECTIVE': true, 's1': true});
expect(divStatic.getAttribute('shadow-class')).toEqual('s1');
expectClass(divBinding).toEqual({'DIRECTIVE': true, 's1': true});
// VE has weird behavior where it calls the @Input('class') with either `class="static` or
// `[class]="dynamic"` but never both. This is determined at compile time. Due to locality
// we don't know if `[class]` is coming if we see `class` only. So we need to combine the
// static and dynamic parts. This results in slightly different calling sequence, but should
// result in the same final DOM.
expect(divBinding.getAttribute('shadow-class')).toEqual(ivyEnabled ? 's1 d1' : 'd1');
});
it('should not feed host style back into shadow input', () => {
@Component({
template: `
<div style="width: 1px;" dir-shadows-class-input></div>
<div style="width: 1px;" [style]=" 'height:1px;' " dir-shadows-class-input></div>
`
})
class Cmp {
}
@Directive({selector: '[dir-shadows-class-input]', host: {'style': 'color: red;'}})
class DirectiveShadowsStyleInput {
constructor(private elementRef: ElementRef) {}
@Input('style')
set style(value: string) {
this.elementRef.nativeElement.setAttribute('shadow-style', value);
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsStyleInput]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const [divStatic, divBinding] = fixture.nativeElement.querySelectorAll('div');
expectStyle(divStatic).toEqual({'color': 'red', 'width': '1px'});
expect(divStatic.getAttribute('shadow-style')).toEqual('width: 1px;');
expectStyle(divBinding).toEqual({'color': 'red', 'width': '1px'});
// VE has weird behavior where it calls the @Input('style') with either `style="static` or
// `[style]="dynamic"` but never both. This is determined at compile time. Due to locality
// we don't know if `[style]` is coming if we see `style` only. So we need to combine the
// static and dynamic parts. This results in slightly different calling sequence, but should
// result in the same final DOM.
expect(divBinding.getAttribute('shadow-style'))
.toEqual(ivyEnabled ? 'width: 1px; height:1px;' : 'height:1px;');
});
onlyInIvy('shadow bindings include static portion')
.it('should bind [class] as input to directive when both static and falsy dynamic values are present',
() => {
@ -375,7 +448,7 @@ describe('styling', () => {
}
@Directive({selector: '[dir-shadows-style-input]'})
class DirectiveShadowsClassInput {
class DirectiveShadowsStyleInput {
constructor(private elementRef: ElementRef) {}
@Input('style')
set style(value: string) {
@ -383,7 +456,7 @@ describe('styling', () => {
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsClassInput]});
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsStyleInput]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
@ -410,7 +483,7 @@ describe('styling', () => {
}
@Directive({selector: '[dir-shadows-style-input]'})
class DirectiveShadowsClassInput {
class DirectiveShadowsStyleInput {
constructor(private elementRef: ElementRef) {}
@Input('style')
set style(value: string) {
@ -418,7 +491,7 @@ describe('styling', () => {
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsClassInput]});
TestBed.configureTestingModule({declarations: [Cmp, DirectiveShadowsStyleInput]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();

6
packages/core/test/render3/styling_next/static_styling_spec.ts
View File

@ -20,7 +20,7 @@ describe('static styling', () => {
tNode = createTNode(null!, null!, TNodeType.Element, 0, '', null);
});
it('should initialize when no attrs', () => {
computeStaticStyling(tNode, []);
computeStaticStyling(tNode, [], true);
expect(tNode.classes).toEqual(null);
expect(tNode.styles).toEqual(null);
});
@ -31,7 +31,7 @@ describe('static styling', () => {
AttributeMarker.Classes, 'my-class', //
AttributeMarker.Styles, 'color', 'red' //
];
computeStaticStyling(tNode, tAttrs);
computeStaticStyling(tNode, tAttrs, true);
expect(tNode.classes).toEqual('my-class');
expect(tNode.styles).toEqual('color: red;');
});
@ -42,7 +42,7 @@ describe('static styling', () => {
AttributeMarker.Classes, 'my-class', 'other', //
AttributeMarker.Styles, 'color', 'red', 'width', '100px' //
];
computeStaticStyling(tNode, tAttrs);
computeStaticStyling(tNode, tAttrs, true);
expect(tNode.classes).toEqual('my-class other');
expect(tNode.styles).toEqual('color: red; width: 100px;');
});