Skip to content

Commit

Permalink
fix(ivy): ensure ngClass works with [class] bindings (#26559)
Browse files Browse the repository at this point in the history
PR Close #26559
  • Loading branch information
matsko committed Oct 25, 2018
1 parent 0cc9842 commit 297dc2b
Show file tree
Hide file tree
Showing 12 changed files with 199 additions and 49 deletions.
4 changes: 3 additions & 1 deletion packages/core/src/render3/i18n.ts
Expand Up @@ -6,8 +6,10 @@
* found in the LICENSE file at https://angular.io/license
*/

import {NO_CHANGE} from '../../src/render3/tokens';

import {assertEqual, assertLessThan} from './assert';
import {NO_CHANGE, _getViewData, adjustBlueprintForNewNode, bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, createNodeAtIndex, getRenderer, load, resetComponentState} from './instructions';
import {_getViewData, adjustBlueprintForNewNode, bindingUpdated, bindingUpdated2, bindingUpdated3, bindingUpdated4, createNodeAtIndex, getRenderer, load, resetComponentState} from './instructions';
import {LContainer, NATIVE, RENDER_PARENT} from './interfaces/container';
import {TElementNode, TNode, TNodeType} from './interfaces/node';
import {RComment, RElement} from './interfaces/renderer';
Expand Down
5 changes: 2 additions & 3 deletions packages/core/src/render3/index.ts
Expand Up @@ -20,9 +20,6 @@ export {CssSelectorList} from './interfaces/projection';

// clang-format off
export {

NO_CHANGE,

bind,
interpolation1,
interpolation2,
Expand Down Expand Up @@ -175,3 +172,5 @@ export {
renderComponent,
whenRendered,
};

export {NO_CHANGE} from './tokens';
70 changes: 48 additions & 22 deletions packages/core/src/render3/instructions.ts
Expand Up @@ -24,13 +24,15 @@ import {PlayerFactory} from './interfaces/player';
import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection';
import {LQueries} from './interfaces/query';
import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {StylingIndex} from './interfaces/styling';
import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTENT_QUERIES, CONTEXT, CurrentMatchesList, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, HOST_NODE, INJECTOR, LViewData, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RootContext, RootContextFlags, SANITIZER, TAIL, TVIEW, TView} from './interfaces/view';
import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
import {appendChild, appendProjectedNode, createTextNode, findComponentView, getLViewChild, getRenderParent, insertView, removeView} from './node_manipulation';
import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher';
import {createStylingContextTemplate, renderStyleAndClassBindings, updateClassProp as updateElementClassProp, updateStyleProp as updateElementStyleProp, updateStylingMap} from './styling/class_and_style_bindings';
import {BoundPlayerFactory} from './styling/player_factory';
import {getStylingContext} from './styling/util';
import {NO_CHANGE} from './tokens';
import {assertDataInRangeInternal, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isContentQueryHost, isDifferent, loadInternal, readPatchedLViewData, stringify} from './util';


Expand Down Expand Up @@ -1337,14 +1339,7 @@ export function elementProperty<T>(
if (value === NO_CHANGE) return;
const element = getNativeByIndex(index, viewData) as RElement | RComment;
const tNode = getTNode(index, viewData);
// if tNode.inputs is undefined, a listener has created outputs, but inputs haven't
// yet been checked
if (tNode && tNode.inputs === undefined) {
// mark inputs as checked
tNode.inputs = generatePropertyAliases(tNode.flags, BindingDirection.Input);
}

const inputData = tNode && tNode.inputs;
const inputData = initializeTNodeInputs(tNode);
let dataValue: PropertyAliasValue|undefined;
if (inputData && (dataValue = inputData[propName])) {
setInputsForProperty(dataValue, value);
Expand Down Expand Up @@ -1543,14 +1538,28 @@ export function elementStyling(
styleDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
styleSanitizer?: StyleSanitizeFn | null): void {
const tNode = previousOrParentTNode;
const inputData = initializeTNodeInputs(tNode);

if (!tNode.stylingTemplate) {
const hasClassInput = inputData && inputData.hasOwnProperty('class') ? true : false;
if (hasClassInput) {
tNode.flags |= TNodeFlags.hasClassInput;
}

// initialize the styling template.
tNode.stylingTemplate =
createStylingContextTemplate(classDeclarations, styleDeclarations, styleSanitizer);
tNode.stylingTemplate = createStylingContextTemplate(
classDeclarations, styleDeclarations, styleSanitizer, hasClassInput);
}

if (styleDeclarations && styleDeclarations.length ||
classDeclarations && classDeclarations.length) {
elementStylingApply(tNode.index - HEADER_OFFSET);
const index = tNode.index - HEADER_OFFSET;
if (delegateToClassInput(tNode)) {
const stylingContext = getStylingContext(index, viewData);
const initialClasses = stylingContext[StylingIndex.PreviousOrCachedMultiClassValue] as string;
setInputsForProperty(tNode.inputs !['class'] !, initialClasses);
}
elementStylingApply(index);
}
}

Expand Down Expand Up @@ -1640,9 +1649,17 @@ export function elementStyleProp(
* removed (unset) from the element's styling.
*/
export function elementStylingMap<T>(
index: number, classes: {[key: string]: any} | string | null,
styles?: {[styleName: string]: any} | null): void {
updateStylingMap(getStylingContext(index, viewData), classes, styles);
index: number, classes: {[key: string]: any} | string | NO_CHANGE | null,
styles?: {[styleName: string]: any} | NO_CHANGE | null): void {
const tNode = getTNode(index, viewData);
const stylingContext = getStylingContext(index, viewData);
if (delegateToClassInput(tNode) && classes !== NO_CHANGE) {
const initialClasses = stylingContext[StylingIndex.PreviousOrCachedMultiClassValue] as string;
const classInputVal =
(initialClasses.length ? (initialClasses + ' ') : '') + (classes as string);
setInputsForProperty(tNode.inputs !['class'] !, classInputVal);
}
updateStylingMap(stylingContext, classes, styles);
}

//////////////////////////
Expand Down Expand Up @@ -2577,14 +2594,6 @@ export function markDirty<T>(component: T) {
//// Bindings & interpolations
///////////////////////////////

export interface NO_CHANGE {
// This is a brand that ensures that this type can never match anything else
brand: 'NO_CHANGE';
}

/** A special value which designates that a value has not changed. */
export const NO_CHANGE = {} as NO_CHANGE;

/**
* Creates a single value binding.
*
Expand Down Expand Up @@ -2871,3 +2880,20 @@ function assertDataNext(index: number, arr?: any[]) {
}

export const CLEAN_PROMISE = _CLEAN_PROMISE;

function initializeTNodeInputs(tNode: TNode | null) {
// If tNode.inputs is undefined, a listener has created outputs, but inputs haven't
// yet been checked.
if (tNode) {
if (tNode.inputs === undefined) {
// mark inputs as checked
tNode.inputs = generatePropertyAliases(tNode.flags, BindingDirection.Input);
}
return tNode.inputs;
}
return null;
}

export function delegateToClassInput(tNode: TNode) {
return tNode.flags & TNodeFlags.hasClassInput;
}
5 changes: 4 additions & 1 deletion packages/core/src/render3/interfaces/node.ts
Expand Up @@ -39,8 +39,11 @@ export const enum TNodeFlags {
/** This bit is set if the node has any content queries */
hasContentQuery = 0b00000000000000000100000000000000,

/** This bit is set if the node has any directives that contain [class properties */
hasClassInput = 0b00000000000000001000000000000000,

/** The index of the first directive on this node is encoded on the most significant bits */
DirectiveStartingIndexShift = 15,
DirectiveStartingIndexShift = 16,
}

/**
Expand Down
24 changes: 14 additions & 10 deletions packages/core/src/render3/interfaces/styling.ts
Expand Up @@ -156,7 +156,7 @@ export interface StylingContext extends Array<InitialStyles|{[key: string]: any}
* The last class value that was interpreted by elementStylingMap. This is cached
* So that the algorithm can exit early incase the value has not changed.
*/
[StylingIndex.PreviousMultiClassValue]: {[key: string]: any}|string|null;
[StylingIndex.PreviousOrCachedMultiClassValue]: {[key: string]: any}|string|null;

/**
* The last style value that was interpreted by elementStylingMap. This is cached
Expand All @@ -181,18 +181,21 @@ export interface InitialStyles extends Array<string|null|boolean> { [0]: null; }
*/
export const enum StylingFlags {
// Implies no configurations
None = 0b0000,
None = 0b00000,
// Whether or not the entry or context itself is dirty
Dirty = 0b0001,
Dirty = 0b00001,
// Whether or not this is a class-based assignment
Class = 0b0010,
Class = 0b00010,
// Whether or not a sanitizer was applied to this property
Sanitize = 0b0100,
Sanitize = 0b00100,
// Whether or not any player builders within need to produce new players
PlayerBuildersDirty = 0b1000,
PlayerBuildersDirty = 0b01000,
// If NgClass is present (or some other class handler) then it will handle the map expressions and
// initial classes
OnlyProcessSingleClasses = 0b10000,
// The max amount of bits used to represent these configuration values
BitCountSize = 4,
// There are only three bits here
BitCountSize = 5,
// There are only five bits here
BitMask = 0b1111
}

Expand All @@ -211,8 +214,9 @@ export const enum StylingIndex {
// Position of where the initial styles are stored in the styling context
// This index must align with HOST, see interfaces/view.ts
ElementPosition = 5,
// Position of where the last string-based CSS class value was stored
PreviousMultiClassValue = 6,
// Position of where the last string-based CSS class value was stored (or a cached version of the
// initial styles when a [class] directive is present)
PreviousOrCachedMultiClassValue = 6,
// Position of where the last string-based CSS class value was stored
PreviousMultiStyleValue = 7,
// Location of single (prop) value entries are stored within the context
Expand Down
35 changes: 26 additions & 9 deletions packages/core/src/render3/styling/class_and_style_bindings.ts
Expand Up @@ -11,6 +11,7 @@ import {BindingStore, BindingType, Player, PlayerBuilder, PlayerFactory, PlayerI
import {Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer';
import {InitialStyles, StylingContext, StylingFlags, StylingIndex} from '../interfaces/styling';
import {LViewData, RootContext} from '../interfaces/view';
import {NO_CHANGE} from '../tokens';
import {getRootContext} from '../util';

import {BoundPlayerFactory} from './player_factory';
Expand Down Expand Up @@ -45,7 +46,7 @@ const EMPTY_OBJ: {[key: string]: any} = {};
export function createStylingContextTemplate(
initialClassDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
initialStyleDeclarations?: (string | boolean | InitialStylingFlags)[] | null,
styleSanitizer?: StyleSanitizeFn | null): StylingContext {
styleSanitizer?: StyleSanitizeFn | null, onlyProcessSingleClasses?: boolean): StylingContext {
const initialStylingValues: InitialStyles = [null];
const context: StylingContext =
createEmptyStylingContext(null, styleSanitizer, initialStylingValues);
Expand Down Expand Up @@ -80,6 +81,7 @@ export function createStylingContextTemplate(
// make where the class offsets begin
context[StylingIndex.ClassOffsetPosition] = totalStyleDeclarations;

const initialStaticClasses: string[]|null = onlyProcessSingleClasses ? [] : null;
if (initialClassDeclarations) {
let hasPassedDeclarations = false;
for (let i = 0; i < initialClassDeclarations.length; i++) {
Expand All @@ -93,6 +95,7 @@ export function createStylingContextTemplate(
const value = initialClassDeclarations[++i] as boolean;
initialStylingValues.push(value);
classesLookup[className] = initialStylingValues.length - 1;
initialStaticClasses && initialStaticClasses.push(className);
} else {
classesLookup[className] = 0;
}
Expand Down Expand Up @@ -143,9 +146,15 @@ export function createStylingContextTemplate(

// there is no initial value flag for the master index since it doesn't
// reference an initial style value
setFlag(context, StylingIndex.MasterFlagPosition, pointers(0, 0, multiStart));
const masterFlag = pointers(0, 0, multiStart) |
(onlyProcessSingleClasses ? StylingFlags.OnlyProcessSingleClasses : 0);
setFlag(context, StylingIndex.MasterFlagPosition, masterFlag);
setContextDirty(context, initialStylingValues.length > 1);

if (initialStaticClasses) {
context[StylingIndex.PreviousOrCachedMultiClassValue] = initialStaticClasses.join(' ');
}

return context;
}

Expand All @@ -164,8 +173,8 @@ export function createStylingContextTemplate(
*/
export function updateStylingMap(
context: StylingContext, classesInput: {[key: string]: any} | string |
BoundPlayerFactory<null|string|{[key: string]: any}>| null,
stylesInput?: {[key: string]: any} | BoundPlayerFactory<null|{[key: string]: any}>|
BoundPlayerFactory<null|string|{[key: string]: any}>| NO_CHANGE | null,
stylesInput?: {[key: string]: any} | BoundPlayerFactory<null|{[key: string]: any}>| NO_CHANGE |
null): void {
stylesInput = stylesInput || null;

Expand All @@ -181,13 +190,14 @@ export function updateStylingMap(
(classesInput as BoundPlayerFactory<{[key: string]: any}|string>) !.value :
classesInput;
const stylesValue = stylesPlayerBuilder ? stylesInput !.value : stylesInput;

// early exit (this is what's done to avoid using ctx.bind() to cache the value)
const ignoreAllClassUpdates = classesValue === context[StylingIndex.PreviousMultiClassValue];
const ignoreAllStyleUpdates = stylesValue === context[StylingIndex.PreviousMultiStyleValue];
const ignoreAllClassUpdates = limitToSingleClasses(context) || classesValue === NO_CHANGE ||
classesValue === context[StylingIndex.PreviousOrCachedMultiClassValue];
const ignoreAllStyleUpdates =
stylesValue === NO_CHANGE || stylesValue === context[StylingIndex.PreviousMultiStyleValue];
if (ignoreAllClassUpdates && ignoreAllStyleUpdates) return;

context[StylingIndex.PreviousMultiClassValue] = classesValue;
context[StylingIndex.PreviousOrCachedMultiClassValue] = classesValue;
context[StylingIndex.PreviousMultiStyleValue] = stylesValue;

let classNames: string[] = EMPTY_ARR;
Expand Down Expand Up @@ -478,6 +488,8 @@ export function renderStyleAndClassBindings(
const native = context[StylingIndex.ElementPosition] !;
const multiStartIndex = getMultiStartIndex(context);
const styleSanitizer = getStyleSanitizer(context);
const onlySingleClasses = limitToSingleClasses(context);

for (let i = StylingIndex.SingleStylesStartPosition; i < context.length;
i += StylingIndex.Size) {
// there is no point in rendering styles that have not changed on screen
Expand All @@ -488,6 +500,7 @@ export function renderStyleAndClassBindings(
const playerBuilder = getPlayerBuilder(context, i);
const isClassBased = flag & StylingFlags.Class ? true : false;
const isInSingleRegion = i < multiStartIndex;
const readInitialValue = !isClassBased || !onlySingleClasses;

let valueToApply: string|boolean|null = value;

Expand All @@ -506,7 +519,7 @@ export function renderStyleAndClassBindings(
// note that this should always be a falsy check since `false` is used
// for both class and style comparisons (styles can't be false and false
// classes are turned off and should therefore defer to their initial values)
if (!valueExists(valueToApply, isClassBased)) {
if (!valueExists(valueToApply, isClassBased) && readInitialValue) {
valueToApply = getInitialValue(context, flag);
}

Expand Down Expand Up @@ -765,6 +778,10 @@ export function isContextDirty(context: StylingContext): boolean {
return isDirty(context, StylingIndex.MasterFlagPosition);
}

export function limitToSingleClasses(context: StylingContext) {
return context[StylingIndex.MasterFlagPosition] & StylingFlags.OnlyProcessSingleClasses;
}

export function setContextDirty(context: StylingContext, isDirtyYes: boolean): void {
setDirty(context, StylingIndex.MasterFlagPosition, isDirtyYes);
}
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/render3/tokens.ts
@@ -0,0 +1,15 @@
/**
* @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
*/

export interface NO_CHANGE {
// This is a brand that ensures that this type can never match anything else
brand: 'NO_CHANGE';
}

/** A special value which designates that a value has not changed. */
export const NO_CHANGE = {} as NO_CHANGE;
Expand Up @@ -446,6 +446,9 @@
{
"name": "defineInjectable"
},
{
"name": "delegateToClassInput"
},
{
"name": "destroyLView"
},
Expand Down Expand Up @@ -740,6 +743,9 @@
{
"name": "hasValueChanged"
},
{
"name": "initializeTNodeInputs"
},
{
"name": "inject"
},
Expand Down Expand Up @@ -833,6 +839,9 @@
{
"name": "leaveView"
},
{
"name": "limitToSingleClasses"
},
{
"name": "listener"
},
Expand Down

0 comments on commit 297dc2b

Please sign in to comment.