diff --git a/packages/core/src/render3/component.ts b/packages/core/src/render3/component.ts index c923209584..ffaefe70cf 100644 --- a/packages/core/src/render3/component.ts +++ b/packages/core/src/render3/component.ts @@ -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) { diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 8f3f80af1f..79f498e065 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -39,8 +39,12 @@ function elementStartFirstCreatePass( resolveDirectives(tView, lView, tNode, getConstant(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); } } diff --git a/packages/core/src/render3/instructions/element_container.ts b/packages/core/src/render3/instructions/element_container.ts index d7e3549c61..6bba7202a2 100644 --- a/packages/core/src/render3/instructions/element_container.ts +++ b/packages/core/src/render3/instructions/element_container.ts @@ -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(tViewConsts, localRefsIndex); diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index 2ebcb9740f..6784c8558f 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -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|undefined|null, // public classes: string|null, // + public classesWithoutHost: string|null, // public residualClasses: KeyValueArray|undefined|null, // public classBindings: TStylingRange, // public styleBindings: TStylingRange, // diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index d171988e88..6a50148716 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -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, diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index 9534975240..65ab0a7db5 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -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 \';\''); diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index a60d7d5f80..ddcf24b6ef 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -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. `
`) + * - There are one or more initial `style`s on an element (e.g. `
`) + * - 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. `
`) + * 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|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. `
`) + * - 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. `
`) + * 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`. * diff --git a/packages/core/src/render3/styling/static_styling.ts b/packages/core/src/render3/styling/static_styling.ts index d27fa55191..b51e79d71c 100644 --- a/packages/core/src/render3/styling/static_styling.ts +++ b/packages/core/src/render3/styling/static_styling.ts @@ -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; } \ No newline at end of file diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 47b39072e4..da62df0a21 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -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: ` +
+
+ ` + }) + 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: ` +
+
+ ` + }) + 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(); diff --git a/packages/core/test/render3/styling_next/static_styling_spec.ts b/packages/core/test/render3/styling_next/static_styling_spec.ts index 5df0dc2bae..31f88eaa33 100644 --- a/packages/core/test/render3/styling_next/static_styling_spec.ts +++ b/packages/core/test/render3/styling_next/static_styling_spec.ts @@ -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;'); });