From b7ff38b1efa313d756f0a592fee08617ffba608b Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Sun, 15 Dec 2019 21:09:02 -0800 Subject: [PATCH] refactor(ivy): Implement `computeStaticStyling` (#34418) The `computeStaticStyling` will be used for computing static styling value during `firstCreatePass`. The function takes into account static styling from the template as well as from the host bindings. The host bindings need to be merged in front of the template so that they have the correct priority. PR Closes #34418 --- integration/_payload-limits.json | 8 +-- .../core/src/render3/instructions/element.ts | 2 +- .../core/src/render3/instructions/styling.ts | 3 +- packages/core/src/render3/interfaces/node.ts | 6 ++- packages/core/src/render3/styling/bindings.ts | 4 +- .../src/render3/styling/static_styling.ts | 43 ++++++++++++++++ .../src/render3/styling/style_binding_list.ts | 44 ++++++++--------- .../core/src/render3/util/styling_utils.ts | 9 +++- packages/core/src/util/stringify.ts | 15 ++++++ .../styling_next/static_styling_spec.ts | 49 +++++++++++++++++++ .../styling_next/style_binding_list_spec.ts | 5 +- packages/core/test/util/stringify_spec.ts | 27 ++++++++++ tools/public_api_guard/core/core.d.ts | 2 +- 13 files changed, 179 insertions(+), 38 deletions(-) create mode 100644 packages/core/src/render3/styling/static_styling.ts create mode 100644 packages/core/test/render3/styling_next/static_styling_spec.ts create mode 100644 packages/core/test/util/stringify_spec.ts diff --git a/integration/_payload-limits.json b/integration/_payload-limits.json index c8bc1991ec..63c5ec4713 100644 --- a/integration/_payload-limits.json +++ b/integration/_payload-limits.json @@ -12,7 +12,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 18271, + "main-es2015": 18214, "polyfills-es2015": 36808 } } @@ -30,7 +30,7 @@ "master": { "uncompressed": { "runtime-es2015": 1485, - "main-es2015": 137141, + "main-es2015": 139487, "polyfills-es2015": 37494 } } @@ -39,7 +39,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 267182, + "main-es2015": 268796, "polyfills-es2015": 36808, "5-es2015": 751 } @@ -49,7 +49,7 @@ "master": { "uncompressed": { "runtime-es2015": 2289, - "main-es2015": 226288, + "main-es2015": 228770, "polyfills-es2015": 36808, "5-es2015": 779 } diff --git a/packages/core/src/render3/instructions/element.ts b/packages/core/src/render3/instructions/element.ts index 599f45eb91..39ec60c5a7 100644 --- a/packages/core/src/render3/instructions/element.ts +++ b/packages/core/src/render3/instructions/element.ts @@ -171,7 +171,7 @@ export function ɵɵelement( } function setDirectiveStylingInput( - context: TStylingContext | StylingMapArray | null, lView: LView, + context: TStylingContext | StylingMapArray | string | null, lView: LView, stylingInputs: (string | number)[], propName: string) { // older versions of Angular treat the input as `null` in the // event that the value does not exist at all. For this reason diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index 68c108bdcf..623eec4382 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -553,7 +553,8 @@ export function registerInitialStylingOnTNode( return hasAdditionalInitialStyling; } -function updateRawValueOnContext(context: TStylingContext | StylingMapArray, value: string) { +function updateRawValueOnContext( + context: TStylingContext | StylingMapArray | string, value: string) { const stylingMapArr = getStylingMapArray(context) !; stylingMapArr[StylingMapArrayIndex.RawValuePosition] = value; } diff --git a/packages/core/src/render3/interfaces/node.ts b/packages/core/src/render3/interfaces/node.ts index 7358cf1bfc..5b0a20cfda 100644 --- a/packages/core/src/render3/interfaces/node.ts +++ b/packages/core/src/render3/interfaces/node.ts @@ -600,7 +600,8 @@ export interface TNode { * are encountered. If and when this happens then the existing `StylingMapArray` value * will be placed into the initial styling slot in the newly created `TStylingContext`. */ - styles: StylingMapArray|TStylingContext|null; + // TODO(misko): `Remove StylingMapArray|TStylingContext|null` in follow up PR. + styles: StylingMapArray|TStylingContext|string|null; /** * A collection of all class bindings and/or static class values for an element. @@ -620,7 +621,8 @@ export interface TNode { * are encountered. If and when this happens then the existing `StylingMapArray` value * will be placed into the initial styling slot in the newly created `TStylingContext`. */ - classes: StylingMapArray|TStylingContext|null; + // TODO(misko): `Remove StylingMapArray|TStylingContext|null` in follow up PR. + classes: StylingMapArray|TStylingContext|string|null; /** * Stores the head/tail index of the class bindings. diff --git a/packages/core/src/render3/styling/bindings.ts b/packages/core/src/render3/styling/bindings.ts index 176f03f050..b3a120ae09 100644 --- a/packages/core/src/render3/styling/bindings.ts +++ b/packages/core/src/render3/styling/bindings.ts @@ -1030,8 +1030,8 @@ export const setStyleAttr = (renderer: Renderer3 | null, native: RElement, value * initial styling values on an element. */ export function renderStylingMap( - renderer: Renderer3, element: RElement, stylingValues: TStylingContext | StylingMapArray | null, - isClassBased: boolean): void { + renderer: Renderer3, element: RElement, + stylingValues: TStylingContext | StylingMapArray | string | null, isClassBased: boolean): void { const stylingMapArr = getStylingMapArray(stylingValues); if (stylingMapArr) { for (let i = StylingMapArrayIndex.ValuesStartPosition; i < stylingMapArr.length; diff --git a/packages/core/src/render3/styling/static_styling.ts b/packages/core/src/render3/styling/static_styling.ts new file mode 100644 index 0000000000..845b16ae22 --- /dev/null +++ b/packages/core/src/render3/styling/static_styling.ts @@ -0,0 +1,43 @@ +/** +* @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 {concatStringsWithSpace} from '../../util/stringify'; +import {assertFirstCreatePass} from '../assert'; +import {AttributeMarker, TAttributes, TNode} from '../interfaces/node'; +import {TVIEW} from '../interfaces/view'; +import {getLView} from '../state'; + +/** + * Compute the static styling (class/style) from `TAttributes`. + * + * This function should be called during `firstCreatePass` only. + * + * @param tNode The `TNode` into which the styling information should be loaded. + * @param attrs `TAttributes` containing the styling information. + */ +export function computeStaticStyling(tNode: TNode, attrs: TAttributes): void { + ngDevMode && assertFirstCreatePass( + getLView()[TVIEW], 'Expecting to be called in first template pass only'); + let styles: string|null = tNode.styles as string | null; + let classes: string|null = tNode.classes as string | 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 + ';'); + } + } + styles !== null && (tNode.styles = styles); + classes !== null && (tNode.classes = classes); +} \ No newline at end of file diff --git a/packages/core/src/render3/styling/style_binding_list.ts b/packages/core/src/render3/styling/style_binding_list.ts index 0b43afca25..1922a52e63 100644 --- a/packages/core/src/render3/styling/style_binding_list.ts +++ b/packages/core/src/render3/styling/style_binding_list.ts @@ -10,12 +10,12 @@ import {stylePropNeedsSanitization, ɵɵsanitizeStyle} from '../../sanitization/ import {assertEqual, throwError} from '../../util/assert'; import {TNode} from '../interfaces/node'; import {SanitizerFn} from '../interfaces/sanitization'; -import {StylingMapArray, TStylingKey, TStylingMapKey, TStylingRange, getTStylingRangeNext, getTStylingRangePrev, getTStylingRangePrevDuplicate, setTStylingRangeNext, setTStylingRangeNextDuplicate, setTStylingRangePrev, setTStylingRangePrevDuplicate, toTStylingRange} from '../interfaces/styling'; +import {TStylingKey, TStylingMapKey, TStylingRange, getTStylingRangeNext, getTStylingRangePrev, getTStylingRangePrevDuplicate, setTStylingRangeNext, setTStylingRangeNextDuplicate, setTStylingRangePrev, setTStylingRangePrevDuplicate, toTStylingRange} from '../interfaces/styling'; import {LView, TData, TVIEW} from '../interfaces/view'; import {getLView} from '../state'; - import {splitClassList, toggleClass} from './class_differ'; import {StyleChangesMap, parseKeyValue, removeStyle} from './style_differ'; +import {getLastParsedKey, parseClassName, parseClassNameNext, parseStyle, parseStyleNext} from './styling_parser'; @@ -254,8 +254,10 @@ export function insertTStylingBinding( // Now we need to update / compute the duplicates. // Starting with our location search towards head (least priority) - markDuplicates(tData, tStylingKey, index, isClassBinding ? tNode.classes : tNode.styles, true); - markDuplicates(tData, tStylingKey, index, null, false); + markDuplicates( + tData, tStylingKey, index, (isClassBinding ? tNode.classes : tNode.styles) as string, true, + isClassBinding); + markDuplicates(tData, tStylingKey, index, '', false, isClassBinding); tBindings = toTStylingRange(tmplHead, tmplTail); if (isClassBinding) { @@ -320,8 +322,8 @@ export function insertTStylingBinding( * @param isPrevDir */ function markDuplicates( - tData: TData, tStylingKey: TStylingKey, index: number, staticValues: StylingMapArray | null, - isPrevDir: boolean) { + tData: TData, tStylingKey: TStylingKey, index: number, staticValues: string, isPrevDir: boolean, + isClassBinding: boolean) { const tStylingAtIndex = tData[index + 1] as TStylingRange; const key: string|null = typeof tStylingKey === 'object' ? tStylingKey.key : tStylingKey; const isMap = key === null; @@ -345,16 +347,19 @@ function markDuplicates( cursor = isPrevDir ? getTStylingRangePrev(tStyleRangeAtCursor) : getTStylingRangeNext(tStyleRangeAtCursor); } - if (staticValues !== null && // If we have static values to search - !foundDuplicate // If we have duplicate don't bother since we are already marked as - // duplicate + if (staticValues !== '' && // If we have static values to search + !foundDuplicate // If we have duplicate don't bother since we are already marked as + // duplicate ) { if (isMap) { // if we are a Map (and we have statics) we must assume duplicate foundDuplicate = true; - } else { - for (let i = 1; foundDuplicate === false && i < staticValues.length; i = i + 2) { - if (staticValues[i] === key) { + } else if (staticValues != null) { + for (let i = isClassBinding ? parseClassName(staticValues) : parseStyle(staticValues); // + i >= 0; // + i = isClassBinding ? parseClassNameNext(staticValues, i) : + parseStyleNext(staticValues, i)) { + if (getLastParsedKey(staticValues) === key) { foundDuplicate = true; break; } @@ -386,8 +391,9 @@ export function flushStyleBinding( // When styling changes we don't have to start at the begging. Instead we start at the change // value and look up the previous concatenation as a starting point going forward. const lastUnchangedValueIndex = getTStylingRangePrev(tStylingRangeAtIndex); - let text = lastUnchangedValueIndex === 0 ? getStaticStylingValue(tNode, isClassBinding) : - lView[lastUnchangedValueIndex + 1] as string; + let text = lastUnchangedValueIndex === 0 ? + ((isClassBinding ? tNode.classes : tNode.styles) as string) : + lView[lastUnchangedValueIndex + 1] as string; let cursor = index; while (cursor !== 0) { const value = lView[cursor]; @@ -400,16 +406,6 @@ export function flushStyleBinding( return text; } -/** - * Retrieves the static value for styling. - * - * @param tNode - * @param isClassBinding - */ -function getStaticStylingValue(tNode: TNode, isClassBinding: Boolean) { - // TODO(misko): implement once we have more code integrated. - return ''; -} /** * Append new styling to the currently concatenated styling text. diff --git a/packages/core/src/render3/util/styling_utils.ts b/packages/core/src/render3/util/styling_utils.ts index 7c87deea1b..a89a69c93e 100644 --- a/packages/core/src/render3/util/styling_utils.ts +++ b/packages/core/src/render3/util/styling_utils.ts @@ -219,8 +219,10 @@ export function hyphenate(value: string): string { * will copy over an initial styling values from the tNode (which are stored as a * `StylingMapArray` on the `tNode.classes` or `tNode.styles` values). */ -export function getStylingMapArray(value: TStylingContext | StylingMapArray | null): +export function getStylingMapArray(value: TStylingContext | StylingMapArray | string | null): StylingMapArray|null { + // TODO(misko): remove after TNode.classes/styles becomes `string` only + if (typeof value === 'string') return null; return isStylingContext(value) ? (value as TStylingContext)[TStylingContextIndex.InitialStylingValuePosition] : value as StylingMapArray; @@ -240,7 +242,10 @@ export function isStylingMapArray(value: any): boolean { (typeof(value as StylingMapArray)[StylingMapArrayIndex.ValuesStartPosition] === 'string'); } -export function getInitialStylingValue(context: TStylingContext | StylingMapArray | null): string { +export function getInitialStylingValue(context: TStylingContext | StylingMapArray | string | null): + string { + // TODO(misko): remove after TNode.classes/styles becomes `string` only + if (typeof context === 'string') return context; const map = getStylingMapArray(context); return map && (map[StylingMapArrayIndex.RawValuePosition] as string | null) || ''; } diff --git a/packages/core/src/util/stringify.ts b/packages/core/src/util/stringify.ts index 138ce95f62..0f789c8082 100644 --- a/packages/core/src/util/stringify.ts +++ b/packages/core/src/util/stringify.ts @@ -36,3 +36,18 @@ export function stringify(token: any): string { const newLineIndex = res.indexOf('\n'); return newLineIndex === -1 ? res : res.substring(0, newLineIndex); } + + +/** + * Concatenates two strings with separator, allocating new strings only when necessary. + * + * @param before before string. + * @param separator separator string. + * @param after after string. + * @returns concatenated string. + */ +export function concatStringsWithSpace(before: string | null, after: string | null): string { + return (before == null || before === '') ? + (after === null ? '' : after) : + ((after == null || after === '') ? before : before + ' ' + after); +} \ No newline at end of file diff --git a/packages/core/test/render3/styling_next/static_styling_spec.ts b/packages/core/test/render3/styling_next/static_styling_spec.ts new file mode 100644 index 0000000000..d9e5d8f2fa --- /dev/null +++ b/packages/core/test/render3/styling_next/static_styling_spec.ts @@ -0,0 +1,49 @@ +/** + * @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 {createTNode} from '@angular/core/src/render3/instructions/shared'; +import {AttributeMarker, TAttributes, TNode, TNodeType} from '@angular/core/src/render3/interfaces/node'; +import {LView} from '@angular/core/src/render3/interfaces/view'; +import {enterView} from '@angular/core/src/render3/state'; +import {computeStaticStyling} from '@angular/core/src/render3/styling/static_styling'; + +describe('static styling', () => { + const mockFirstCreatePassLView: LView = [null, {firstCreatePass: true}] as any; + let tNode !: TNode; + beforeEach(() => { + enterView(mockFirstCreatePassLView, null); + tNode = createTNode(null !, null !, TNodeType.Element, 0, '', null); + }); + it('should initialize when no attrs', () => { + computeStaticStyling(tNode, []); + expect(tNode.classes).toEqual(null); + expect(tNode.styles).toEqual(null); + }); + + it('should initialize from attrs', () => { + const tAttrs: TAttributes = [ + 'ignore', // + AttributeMarker.Classes, 'my-class', // + AttributeMarker.Styles, 'color', 'red' // + ]; + computeStaticStyling(tNode, tAttrs); + expect(tNode.classes).toEqual('my-class'); + expect(tNode.styles).toEqual('color: red;'); + }); + + it('should initialize from attrs when multiple', () => { + const tAttrs: TAttributes = [ + 'ignore', // + AttributeMarker.Classes, 'my-class', 'other', // + AttributeMarker.Styles, 'color', 'red', 'width', '100px' // + ]; + computeStaticStyling(tNode, tAttrs); + expect(tNode.classes).toEqual('my-class other'); + expect(tNode.styles).toEqual('color: red; width: 100px;'); + }); +}); \ No newline at end of file diff --git a/packages/core/test/render3/styling_next/style_binding_list_spec.ts b/packages/core/test/render3/styling_next/style_binding_list_spec.ts index 335034e4af..260427c060 100644 --- a/packages/core/test/render3/styling_next/style_binding_list_spec.ts +++ b/packages/core/test/render3/styling_next/style_binding_list_spec.ts @@ -115,6 +115,7 @@ describe('TNode styling linked list', () => { // ɵɵstyleProp('color', '#008'); // Binding index: 30 const tNode = createTNode(null !, null !, TNodeType.Element, 0, '', null); + tNode.styles = ''; const tData: TData = newArray(32, null); const STYLE = STYLE_MAP_STYLING_KEY; @@ -408,7 +409,7 @@ describe('TNode styling linked list', () => { it('should mark duplicate on static fields', () => { const tNode = createTNode(null !, null !, TNodeType.Element, 0, '', null); - tNode.styles = [null, 'color', 'blue']; + tNode.styles = 'color: blue;'; const tData: TData = [null, null]; insertTStylingBinding(tData, tNode, 'width', 2, false, false); expectPriorityOrder(tData, tNode, false).toEqual([ @@ -636,6 +637,8 @@ class StylingFixture { lView: LView = [null, null !] as any; tNode: TNode = createTNode(null !, null !, TNodeType.Element, 0, '', null); constructor(bindingSources: TStylingKey[][], public isClassBinding: boolean) { + this.tNode.classes = ''; + this.tNode.styles = ''; let bindingIndex = this.tData.length; for (let i = 0; i < bindingSources.length; i++) { const bindings = bindingSources[i]; diff --git a/packages/core/test/util/stringify_spec.ts b/packages/core/test/util/stringify_spec.ts new file mode 100644 index 0000000000..e5ce710f6c --- /dev/null +++ b/packages/core/test/util/stringify_spec.ts @@ -0,0 +1,27 @@ +/** + * @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 {concatStringsWithSpace} from '@angular/core/src/util/stringify'; + +describe('stringify', () => { + describe('concatStringsWithSpace', () => { + it('should concat with null', () => { + expect(concatStringsWithSpace(null, null)).toEqual(''); + expect(concatStringsWithSpace('a', null)).toEqual('a'); + expect(concatStringsWithSpace(null, 'b')).toEqual('b'); + }); + + it('should concat when empty', () => { + expect(concatStringsWithSpace('', '')).toEqual(''); + expect(concatStringsWithSpace('a', '')).toEqual('a'); + expect(concatStringsWithSpace('', 'b')).toEqual('b'); + }); + + it('should concat when not empty', + () => { expect(concatStringsWithSpace('before', 'after')).toEqual('before after'); }); + }); +}); diff --git a/tools/public_api_guard/core/core.d.ts b/tools/public_api_guard/core/core.d.ts index efeea3b1f1..6516edcf3a 100644 --- a/tools/public_api_guard/core/core.d.ts +++ b/tools/public_api_guard/core/core.d.ts @@ -786,7 +786,7 @@ export declare const ɵɵdefineDirective: (directiveDefinition: { features?: DirectiveDefFeature[] | undefined; hostBindings?: HostBindingsFunction | undefined; hostVars?: number | undefined; - hostAttrs?: (string | (string | SelectorFlags)[] | AttributeMarker)[] | undefined; + hostAttrs?: TAttributes | undefined; contentQueries?: ContentQueriesFunction | undefined; viewQuery?: ViewQueriesFunction | null | undefined; exportAs?: string[] | undefined;