fix(ivy): ensure template styles/classes are applied before directives are instantiated (#29269)

Angular Ivy interprets inline static style/class attribute values as instructions that
are processed whilst an element gets created. Because these inline style values are
referenced by style/class bindings, their inline style values are applied at a later
stage. Despite them being eventually applied, their values should be applied earlier
before any directives are instantiated so that directive code can rely on any inline
style/class changes.

This patch ensures that all static style/class attribute values are applied (rendered)
on the element before directives are instantiated.

Jira Issue: FW-1133

PR Close #29269
This commit is contained in:
Matias Niemelä 2019-03-12 14:14:08 -07:00
parent 1a9ab2727e
commit 1877e6c3f8
8 changed files with 125 additions and 62 deletions

View File

@ -619,10 +619,13 @@ export function elementStart(
ngDevMode && ngDevMode.rendererCreateElement++;
const native = elementCreate(name);
const renderer = lView[RENDERER];
ngDevMode && assertDataInRange(lView, index - 1);
const tNode = createNodeAtIndex(index, TNodeType.Element, native !, name, attrs || null);
let initialStylesIndex = 0;
let initialClassesIndex = 0;
if (attrs) {
const lastAttrIndex = setUpAttributes(native, attrs);
@ -640,6 +643,14 @@ export function elementStart(
tNode.stylingTemplate = initializeStaticStylingContext(attrs, stylingAttrsStartIndex);
}
}
if (tNode.stylingTemplate) {
// the initial style/class values are rendered immediately after having been
// initialized into the context so the element styling is ready when directives
// are initialized (since they may read style/class values in their constructor)
initialStylesIndex = renderInitialStyles(native, tNode.stylingTemplate, renderer);
initialClassesIndex = renderInitialClasses(native, tNode.stylingTemplate, renderer);
}
}
appendChild(native, tNode, lView);
@ -667,11 +678,11 @@ export function elementStart(
}
}
// There is no point in rendering styles when a class directive is present since
// it will take that over for us (this will be removed once #FW-882 is in).
// we render the styling again below in case any directives have set any `style` and/or
// `class` host attribute values...
if (tNode.stylingTemplate) {
renderInitialClasses(native, tNode.stylingTemplate, lView[RENDERER]);
renderInitialStyles(native, tNode.stylingTemplate, lView[RENDERER]);
renderInitialClasses(native, tNode.stylingTemplate, renderer, initialClassesIndex);
renderInitialStyles(native, tNode.stylingTemplate, renderer, initialStylesIndex);
}
const currentQueries = lView[QUERIES];

View File

@ -470,6 +470,12 @@ export const enum InitialStylingValuesIndex {
*/
DirectiveOwnerOffset = 2,
/**
* The first bit set aside to mark if the initial style was already rendere
*/
AppliedFlagBitPosition = 0b0,
AppliedFlagBitLength = 1,
/**
* The total size for each style/class entry (prop + value + directiveOwner)
*/

View File

@ -139,47 +139,53 @@ function patchInitialStylingValue(
}
/**
* Runs through the initial style data present in the context and renders
* them via the renderer on the element.
*/
export function renderInitialStyles(
element: RElement, context: StylingContext, renderer: Renderer3) {
const initialStyles = context[StylingIndex.InitialStyleValuesPosition];
renderInitialStylingValues(element, renderer, initialStyles, false);
}
/**
* Runs through the initial class data present in the context and renders
* them via the renderer on the element.
* Runs through the initial class values present in the provided
* context and renders them via the provided renderer on the element.
*
* @param element the element the styling will be applied to
* @param context the source styling context which contains the initial class values
* @param renderer the renderer instance that will be used to apply the class
* @returns the index that the classes were applied up until
*/
export function renderInitialClasses(
element: RElement, context: StylingContext, renderer: Renderer3) {
element: RElement, context: StylingContext, renderer: Renderer3, startIndex?: number): number {
const initialClasses = context[StylingIndex.InitialClassValuesPosition];
renderInitialStylingValues(element, renderer, initialClasses, true);
let i = startIndex || InitialStylingValuesIndex.KeyValueStartPosition;
while (i < initialClasses.length) {
const value = initialClasses[i + InitialStylingValuesIndex.ValueOffset];
if (value) {
setClass(
element, initialClasses[i + InitialStylingValuesIndex.PropOffset] as string, true,
renderer, null);
}
i += InitialStylingValuesIndex.Size;
}
return i;
}
/**
* This is a helper function designed to render each entry present within the
* provided list of initialStylingValues.
* Runs through the initial styles values present in the provided
* context and renders them via the provided renderer on the element.
*
* @param element the element the styling will be applied to
* @param context the source styling context which contains the initial class values
* @param renderer the renderer instance that will be used to apply the class
* @returns the index that the styles were applied up until
*/
function renderInitialStylingValues(
element: RElement, renderer: Renderer3, initialStylingValues: InitialStylingValues,
isEntryClassBased: boolean) {
for (let i = InitialStylingValuesIndex.KeyValueStartPosition; i < initialStylingValues.length;
i += InitialStylingValuesIndex.Size) {
const value = initialStylingValues[i + InitialStylingValuesIndex.ValueOffset];
export function renderInitialStyles(
element: RElement, context: StylingContext, renderer: Renderer3, startIndex?: number) {
const initialStyles = context[StylingIndex.InitialStyleValuesPosition];
let i = startIndex || InitialStylingValuesIndex.KeyValueStartPosition;
while (i < initialStyles.length) {
const value = initialStyles[i + InitialStylingValuesIndex.ValueOffset];
if (value) {
if (isEntryClassBased) {
setClass(
element, initialStylingValues[i + InitialStylingValuesIndex.PropOffset] as string, true,
renderer, null);
} else {
setStyle(
element, initialStylingValues[i + InitialStylingValuesIndex.PropOffset] as string,
element, initialStyles[i + InitialStylingValuesIndex.PropOffset] as string,
value as string, renderer, null);
}
i += InitialStylingValuesIndex.Size;
}
}
return i;
}
export function allowNewBindingsForStylingContext(context: StylingContext): boolean {

View File

@ -5,7 +5,7 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Component, HostBinding} from '@angular/core';
import {Component, Directive, ElementRef, HostBinding} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {ivyEnabled, onlyInIvy} from '@angular/private/testing';
@ -96,4 +96,61 @@ describe('acceptance integration tests', () => {
expect(element.classList.contains('foo')).toBeTruthy();
expect(element.classList.contains('baz')).toBeTruthy();
});
it('should render inline style and class attribute values on the element before a directive is instantiated',
() => {
@Component({
template: `
<div directive-expecting-styling style="width:200px" class="abc xyz"></div>
`
})
class Cmp {
}
@Directive({selector: '[directive-expecting-styling]'})
class DirectiveExpectingStyling {
constructor(elm: ElementRef) {
const native = elm.nativeElement;
native.setAttribute('data-captured-width', native.style.width);
native.setAttribute('data-captured-classes', native.className);
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveExpectingStyling]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const element = fixture.nativeElement.querySelector('div');
expect(element.style.width).toEqual('200px');
expect(element.getAttribute('data-captured-width')).toEqual('200px');
expect(element.className.trim()).toEqual('abc xyz');
expect(element.getAttribute('data-captured-classes')).toEqual('abc xyz');
});
it('should only render the same initial styling values once before a directive runs', () => {
@Component({
template: `
<div directive-expecting-styling style="width:200px" class="abc"></div>
`
})
class Cmp {
}
@Directive({selector: '[directive-expecting-styling]'})
class DirectiveExpectingStyling {
constructor(elm: ElementRef) {
const native = elm.nativeElement;
native.style.width = '300px';
native.classList.remove('abc');
}
}
TestBed.configureTestingModule({declarations: [Cmp, DirectiveExpectingStyling]});
const fixture = TestBed.createComponent(Cmp);
fixture.detectChanges();
const element = fixture.nativeElement.querySelector('div');
expect(element.style.width).toEqual('300px');
expect(element.classList.contains('abc')).toBeFalsy();
});
});

View File

@ -605,9 +605,6 @@
{
"name": "renderInitialStyles"
},
{
"name": "renderInitialStylingValues"
},
{
"name": "renderStringify"
},

View File

@ -425,9 +425,6 @@
{
"name": "renderInitialStyles"
},
{
"name": "renderInitialStylingValues"
},
{
"name": "renderStringify"
},

View File

@ -1178,9 +1178,6 @@
{
"name": "renderInitialStyles"
},
{
"name": "renderInitialStylingValues"
},
{
"name": "renderStringify"
},

View File

@ -104,14 +104,6 @@ window.testBlocklist = {
"MatSnackBar with TemplateRef should be able to open a snack bar using a TemplateRef": {
"error": "Error: Expected ' Fries Pizza ' to contain 'Pasta'.",
"notes": "Breaking change: Change detection follows insertion tree only, not declaration tree (MatSnackBarContainer is OnPush)"
},
"MatTooltip special cases should clear the `user-select` when a tooltip is set on a text field": {
"error": "Error: Expected 'none' to be falsy.",
"notes": "FW-1133: Inline styles are not applied before constructor is run"
},
"MatTooltip special cases should clear the `-webkit-user-drag` on draggable elements": {
"error": "Error: Expected 'none' to be falsy.",
"notes": "FW-1133: Inline styles are not applied before constructor is run"
}
};
// clang-format on