refactor(ivy): change styling to use programmatic API on updates (#34804)
Previously we would write to class/style as strings `element.className` and `element.style.cssText`. Turns out that approach is good for initial render but not good for updates. Updates using this approach are problematic because we have to check to see if there was an out of bound write to style and than perform reconciliation. This also requires the browser to bring up CSS parser which is expensive. Another problem with old approach is that we had to queue the DOM writes and flush them twice. Once on element advance instruction and once in `hostBindings`. The double flushing is expensive but it also means that a directive can observe that styles are not yet written (they are written after directive executes.) The new approach uses `element.classList.add/remove` and `element.style.setProperty/removeProperty` API for updates only (it continues to use `element.className` and `element.style.cssText` for initial render as it is cheaper.) The other change is that the styling changes are applied immediately (no queueing). This means that it is the instruction which computes priority. In some circumstances it may result in intermediate writes which are than overwritten with new value. (This should be rare) Overall this change deletes most of the previous code and replaces it with new simplified implement. The simplification results in code savings. PR Close #34804
This commit is contained in:
Miško Hevery
@ -11,7 +11,7 @@ import {createTNode, createTView} from '@angular/core/src/render3/instructions/s
|
||||
import {TNodeType} from '@angular/core/src/render3/interfaces/node';
|
||||
import {LView, TView, TViewType} from '@angular/core/src/render3/interfaces/view';
|
||||
import {enterView, leaveView} from '@angular/core/src/render3/state';
|
||||
import {CLASS_MAP_STYLING_KEY, STYLE_MAP_STYLING_KEY, insertTStylingBinding} from '@angular/core/src/render3/styling/style_binding_list';
|
||||
import {insertTStylingBinding} from '@angular/core/src/render3/styling/style_binding_list';
|
||||
|
||||
|
||||
describe('lView_debug', () => {
|
||||
@ -98,13 +98,13 @@ describe('lView_debug', () => {
|
||||
}
|
||||
]);
|
||||
|
||||
insertTStylingBinding(tView.data, tNode, STYLE_MAP_STYLING_KEY, 6, true, true);
|
||||
insertTStylingBinding(tView.data, tNode, CLASS_MAP_STYLING_KEY, 8, true, false);
|
||||
insertTStylingBinding(tView.data, tNode, null, 6, true, true);
|
||||
insertTStylingBinding(tView.data, tNode, null, 8, true, false);
|
||||
|
||||
expect(tNode.styleBindings_).toEqual([
|
||||
null, {
|
||||
index: 8,
|
||||
key: CLASS_MAP_STYLING_KEY,
|
||||
key: null,
|
||||
isTemplate: false,
|
||||
prevDuplicate: false,
|
||||
nextDuplicate: true,
|
||||
@ -124,7 +124,7 @@ describe('lView_debug', () => {
|
||||
expect(tNode.classBindings_).toEqual([
|
||||
'STATIC', {
|
||||
index: 6,
|
||||
key: STYLE_MAP_STYLING_KEY,
|
||||
key: null,
|
||||
isTemplate: false,
|
||||
prevDuplicate: true,
|
||||
nextDuplicate: true,
|
||||
|
||||
53
packages/core/test/render3/instructions/shared_spec.ts
Normal file
53
packages/core/test/render3/instructions/shared_spec.ts
Normal file
@ -0,0 +1,53 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* 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 {createLView, createTNode, createTView} from '@angular/core/src/render3/instructions/shared';
|
||||
import {TNodeType} from '@angular/core/src/render3/interfaces/node';
|
||||
import {domRendererFactory3} from '@angular/core/src/render3/interfaces/renderer';
|
||||
import {HEADER_OFFSET, LViewFlags, TVIEW, TViewType} from '@angular/core/src/render3/interfaces/view';
|
||||
import {enterView, getBindingRoot, getLView, setBindingIndex} from '@angular/core/src/render3/state';
|
||||
|
||||
|
||||
|
||||
/**
|
||||
* Setups a simple `LView` so that it is possible to do unit tests on instructions.
|
||||
*
|
||||
* ```
|
||||
* describe('styling', () => {
|
||||
* beforeEach(enterViewWithOneDiv);
|
||||
* afterEach(leaveView);
|
||||
*
|
||||
* it('should ...', () => {
|
||||
* expect(getLView()).toBeDefined();
|
||||
* const div = getNativeByIndex(1, getLView());
|
||||
* });
|
||||
* });
|
||||
* ```
|
||||
*/
|
||||
export function enterViewWithOneDiv() {
|
||||
const renderer = domRendererFactory3.createRenderer(null, null);
|
||||
const div = renderer.createElement('div');
|
||||
const tView =
|
||||
createTView(TViewType.Component, -1, emptyTemplate, 1, 10, null, null, null, null, null);
|
||||
const tNode = tView.firstChild = createTNode(tView, null !, TNodeType.Element, 0, 'div', null);
|
||||
const lView = createLView(
|
||||
null, tView, null, LViewFlags.CheckAlways, null, null, domRendererFactory3, renderer, null,
|
||||
null);
|
||||
lView[0 + HEADER_OFFSET] = div;
|
||||
tView.data[0 + HEADER_OFFSET] = tNode;
|
||||
enterView(lView, tNode);
|
||||
}
|
||||
|
||||
export function clearFirstUpdatePass() {
|
||||
getLView()[TVIEW].firstUpdatePass = false;
|
||||
}
|
||||
export function rewindBindingIndex() {
|
||||
setBindingIndex(getBindingRoot());
|
||||
}
|
||||
|
||||
function emptyTemplate() {}
|
||||
358
packages/core/test/render3/instructions/styling_spec.ts
Normal file
358
packages/core/test/render3/instructions/styling_spec.ts
Normal file
@ -0,0 +1,358 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright Google Inc. All Rights Reserved.
|
||||
*
|
||||
* 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 {classStringParser, initializeStylingStaticArrayMap, styleStringParser, toStylingArrayMap, ɵɵclassProp, ɵɵstyleMap, ɵɵstyleProp, ɵɵstyleSanitizer} from '@angular/core/src/render3/instructions/styling';
|
||||
import {AttributeMarker} from '@angular/core/src/render3/interfaces/node';
|
||||
import {TVIEW} from '@angular/core/src/render3/interfaces/view';
|
||||
import {getLView, leaveView} from '@angular/core/src/render3/state';
|
||||
import {getNativeByIndex} from '@angular/core/src/render3/util/view_utils';
|
||||
import {bypassSanitizationTrustStyle} from '@angular/core/src/sanitization/bypass';
|
||||
import {ɵɵsanitizeStyle} from '@angular/core/src/sanitization/sanitization';
|
||||
import {arrayMapSet} from '@angular/core/src/util/array_utils';
|
||||
import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode';
|
||||
import {getElementClasses, getElementStyles} from '@angular/core/testing/src/styling';
|
||||
import {expect} from '@angular/core/testing/src/testing_internal';
|
||||
|
||||
import {clearFirstUpdatePass, enterViewWithOneDiv, rewindBindingIndex} from './shared_spec';
|
||||
|
||||
describe('styling', () => {
|
||||
beforeEach(enterViewWithOneDiv);
|
||||
afterEach(leaveView);
|
||||
|
||||
let div !: HTMLElement;
|
||||
beforeEach(() => div = getNativeByIndex(0, getLView()) as HTMLElement);
|
||||
|
||||
it('should do set basic style', () => {
|
||||
ɵɵstyleProp('color', 'red');
|
||||
expectStyle(div).toEqual({color: 'red'});
|
||||
});
|
||||
|
||||
it('should search across multiple instructions backwards', () => {
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleProp('color', undefined);
|
||||
ɵɵstyleProp('color', 'blue');
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleProp('color', undefined);
|
||||
ɵɵstyleProp('color', undefined);
|
||||
expectStyle(div).toEqual({color: 'red'});
|
||||
});
|
||||
|
||||
it('should search across multiple instructions forwards', () => {
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleProp('color', 'green');
|
||||
ɵɵstyleProp('color', 'blue');
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'white');
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
});
|
||||
|
||||
it('should set style based on priority', () => {
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleProp('color', 'blue'); // Higher priority, should win.
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
// The intermediate value has to set since it does not know if it is last one.
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(2);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
clearFirstUpdatePass();
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'red'); // no change
|
||||
ɵɵstyleProp('color', 'green'); // change to green
|
||||
expectStyle(div).toEqual({color: 'green'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'black'); // First binding update
|
||||
expectStyle(div).toEqual({color: 'green'}); // Green still has priority.
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'black'); // no change
|
||||
ɵɵstyleProp('color', undefined); // Clear second binding
|
||||
expectStyle(div).toEqual({color: 'black'}); // default to first binding
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', null);
|
||||
expectStyle(div).toEqual({}); // default to first binding
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(0);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(1);
|
||||
});
|
||||
|
||||
it('should set class based on priority', () => {
|
||||
ɵɵclassProp('foo', false);
|
||||
ɵɵclassProp('foo', true); // Higher priority, should win.
|
||||
expectClass(div).toEqual({foo: true});
|
||||
expect(ngDevMode !.rendererAddClass).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
clearFirstUpdatePass();
|
||||
rewindBindingIndex();
|
||||
ɵɵclassProp('foo', false); // no change
|
||||
ɵɵclassProp('foo', undefined); // change (have no opinion)
|
||||
expectClass(div).toEqual({});
|
||||
expect(ngDevMode !.rendererAddClass).toEqual(0);
|
||||
expect(ngDevMode !.rendererRemoveClass).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵclassProp('foo', false); // no change
|
||||
ɵɵclassProp('foo', 'truthy' as any);
|
||||
expectClass(div).toEqual({foo: true});
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵclassProp('foo', true); // change
|
||||
ɵɵclassProp('foo', undefined); // change
|
||||
expectClass(div).toEqual({foo: true});
|
||||
});
|
||||
|
||||
describe('styleMap', () => {
|
||||
it('should work with maps', () => {
|
||||
ɵɵstyleMap({});
|
||||
expectStyle(div).toEqual({});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(0);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap({color: 'blue'});
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap({color: 'red'});
|
||||
expectStyle(div).toEqual({color: 'red'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap({color: null, width: '100px'});
|
||||
expectStyle(div).toEqual({width: '100px'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
});
|
||||
|
||||
it('should work with object literal and strings', () => {
|
||||
ɵɵstyleMap('');
|
||||
expectStyle(div).toEqual({});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(0);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap('color: blue');
|
||||
expectStyle(div).toEqual({color: 'blue'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap('color: red');
|
||||
expectStyle(div).toEqual({color: 'red'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(0);
|
||||
ngDevModeResetPerfCounters();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap('width: 100px');
|
||||
expectStyle(div).toEqual({width: '100px'});
|
||||
expect(ngDevMode !.rendererSetStyle).toEqual(1);
|
||||
expect(ngDevMode !.rendererRemoveStyle).toEqual(1);
|
||||
ngDevModeResetPerfCounters();
|
||||
});
|
||||
|
||||
it('should collaborate with properties', () => {
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleMap({color: 'blue', width: '100px'});
|
||||
expectStyle(div).toEqual({color: 'blue', width: '100px'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('color', 'red');
|
||||
ɵɵstyleMap({width: '200px'});
|
||||
expectStyle(div).toEqual({color: 'red', width: '200px'});
|
||||
});
|
||||
|
||||
it('should collaborate with other maps', () => {
|
||||
ɵɵstyleMap('color: red');
|
||||
ɵɵstyleMap({color: 'blue', width: '100px'});
|
||||
expectStyle(div).toEqual({color: 'blue', width: '100px'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap('color: red');
|
||||
ɵɵstyleMap({width: '200px'});
|
||||
expectStyle(div).toEqual({color: 'red', width: '200px'});
|
||||
});
|
||||
|
||||
describe('suffix', () => {
|
||||
it('should append suffix', () => {
|
||||
ɵɵstyleProp('width', 200, 'px');
|
||||
ɵɵstyleProp('width', 100, 'px');
|
||||
expectStyle(div).toEqual({width: '100px'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('width', 200, 'px');
|
||||
ɵɵstyleProp('width', undefined, 'px');
|
||||
expectStyle(div).toEqual({width: '200px'});
|
||||
});
|
||||
|
||||
it('should append suffix and non-suffix bindings', () => {
|
||||
ɵɵstyleProp('width', 200, 'px');
|
||||
ɵɵstyleProp('width', '100px');
|
||||
expectStyle(div).toEqual({width: '100px'});
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('width', 200, 'px');
|
||||
ɵɵstyleProp('width', undefined, 'px');
|
||||
expectStyle(div).toEqual({width: '200px'});
|
||||
});
|
||||
});
|
||||
|
||||
describe('sanitization', () => {
|
||||
it('should sanitize property', () => {
|
||||
ɵɵstyleSanitizer(ɵɵsanitizeStyle);
|
||||
ɵɵstyleProp('background', 'url("javascript:/unsafe")');
|
||||
expect(div.style.getPropertyValue('background')).not.toContain('javascript');
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleProp('background', bypassSanitizationTrustStyle('url("javascript:/trusted")'));
|
||||
expect(div.style.getPropertyValue('background')).toContain('url("javascript:/trusted")');
|
||||
});
|
||||
|
||||
it('should sanitize map', () => {
|
||||
ɵɵstyleSanitizer(ɵɵsanitizeStyle);
|
||||
ɵɵstyleMap('background: url("javascript:/unsafe")');
|
||||
expect(div.style.getPropertyValue('background')).not.toContain('javascript');
|
||||
|
||||
clearFirstUpdatePass();
|
||||
|
||||
rewindBindingIndex();
|
||||
ɵɵstyleMap({'background': bypassSanitizationTrustStyle('url("javascript:/trusted")')});
|
||||
expect(div.style.getPropertyValue('background')).toContain('url("javascript:/trusted")');
|
||||
});
|
||||
});
|
||||
|
||||
describe('populateStylingStaticArrayMap', () => {
|
||||
it('should initialize to null if no mergedAttrs', () => {
|
||||
const tNode = getLView()[TVIEW].firstChild !;
|
||||
expect(tNode.stylesMap).toEqual(undefined);
|
||||
expect(tNode.classesMap).toEqual(undefined);
|
||||
initializeStylingStaticArrayMap(tNode);
|
||||
expect(tNode.stylesMap).toEqual(null);
|
||||
expect(tNode.classesMap).toEqual(null);
|
||||
});
|
||||
|
||||
it('should initialize from mergeAttrs', () => {
|
||||
const tNode = getLView()[TVIEW].firstChild !;
|
||||
expect(tNode.stylesMap).toEqual(undefined);
|
||||
expect(tNode.classesMap).toEqual(undefined);
|
||||
tNode.mergedAttrs = [
|
||||
'ignore', 'value', //
|
||||
AttributeMarker.Classes, 'foo', 'bar', //
|
||||
AttributeMarker.Styles, 'width', '0', 'color', 'red', //
|
||||
];
|
||||
initializeStylingStaticArrayMap(tNode);
|
||||
expect(tNode.classesMap).toEqual(['bar', true, 'foo', true] as any);
|
||||
expect(tNode.stylesMap).toEqual(['color', 'red', 'width', '0'] as any);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
describe('toStylingArray', () => {
|
||||
describe('falsy', () => {
|
||||
it('should return empty ArrayMap', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, '')).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, null)).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, undefined)).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, [])).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, {})).toEqual([] as any);
|
||||
});
|
||||
describe('string', () => {
|
||||
it('should parse classes', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, classStringParser, ' ')).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, classStringParser, ' X A ')).toEqual([
|
||||
'A', true, 'X', true
|
||||
] as any);
|
||||
});
|
||||
it('should parse styles', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, styleStringParser, ' ')).toEqual([] as any);
|
||||
expect(toStylingArrayMap(arrayMapSet, styleStringParser, 'B:b;A:a')).toEqual([
|
||||
'A', 'a', 'B', 'b'
|
||||
] as any);
|
||||
});
|
||||
});
|
||||
describe('array', () => {
|
||||
it('should parse', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, ['X', 'A'])).toEqual([
|
||||
'A', true, 'X', true
|
||||
] as any);
|
||||
});
|
||||
});
|
||||
describe('object', () => {
|
||||
it('should parse', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, {X: 'x', A: 'a'})).toEqual([
|
||||
'A', 'a', 'X', 'x'
|
||||
] as any);
|
||||
});
|
||||
});
|
||||
describe('Map', () => {
|
||||
it('should parse', () => {
|
||||
expect(toStylingArrayMap(
|
||||
arrayMapSet, null !, new Map<string, string>([['X', 'x'], ['A', 'a']])))
|
||||
.toEqual(['A', 'a', 'X', 'x'] as any);
|
||||
});
|
||||
});
|
||||
describe('Iterable', () => {
|
||||
it('should parse', () => {
|
||||
expect(toStylingArrayMap(arrayMapSet, null !, new Set<string>(['X', 'A']))).toEqual([
|
||||
'A', true, 'X', true
|
||||
] as any);
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
function expectStyle(element: HTMLElement) {
|
||||
return expect(getElementStyles(element));
|
||||
}
|
||||
|
||||
function expectClass(element: HTMLElement) {
|
||||
return expect(getElementClasses(element));
|
||||
}
|
||||
Reference in New Issue
Block a user