diff --git a/packages/common/src/directives/ng_class.ts b/packages/common/src/directives/ng_class.ts index a160ac95faf86..fe6b8e6ff44cd 100644 --- a/packages/common/src/directives/ng_class.ts +++ b/packages/common/src/directives/ng_class.ts @@ -34,7 +34,7 @@ export const ngClassDirectiveDef__POST_R3__ = ɵɵdefineDirective({ selectors: null as any, hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) { if (rf & ɵRenderFlags.Create) { - ɵɵallocHostVars(1); + ɵɵallocHostVars(2); } if (rf & ɵRenderFlags.Update) { ɵɵclassMap(ctx.getValue()); diff --git a/packages/common/src/directives/ng_style.ts b/packages/common/src/directives/ng_style.ts index 6d728f1b5674f..97670a906ef15 100644 --- a/packages/common/src/directives/ng_style.ts +++ b/packages/common/src/directives/ng_style.ts @@ -35,7 +35,7 @@ export const ngStyleDirectiveDef__POST_R3__ = ɵɵdefineDirective({ selectors: null as any, hostBindings: function(rf: ɵRenderFlags, ctx: any, elIndex: number) { if (rf & ɵRenderFlags.Create) { - ɵɵallocHostVars(1); + ɵɵallocHostVars(2); } if (rf & ɵRenderFlags.Update) { ɵɵstyleMap(ctx.getValue()); diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index cf8c3d99afa61..5ae7759b70389 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -436,7 +436,7 @@ describe('compiler compliance: styling', () => { const template = ` … decls: 1, - vars: 2, + vars: 3, template: function MyComponentWithInterpolation_Template(rf, $ctx$) { if (rf & 1) { $r3$.ɵɵelement(0, "div"); @@ -447,7 +447,7 @@ describe('compiler compliance: styling', () => { } … decls: 1, - vars: 3, + vars: 4, template: function MyComponentWithMuchosInterpolation_Template(rf, $ctx$) { if (rf & 1) { $r3$.ɵɵelement(0, "div"); @@ -458,7 +458,7 @@ describe('compiler compliance: styling', () => { } … decls: 1, - vars: 1, + vars: 2, template: function MyComponentWithoutInterpolation_Template(rf, $ctx$) { if (rf & 1) { $r3$.ɵɵelement(0, "div"); @@ -506,7 +506,7 @@ describe('compiler compliance: styling', () => { type: MyComponent, selectors:[["my-component"]], decls: 1, - vars: 4, + vars: 5, consts: [[${AttributeMarker.Styles}, "opacity", "1"]], template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { @@ -700,7 +700,7 @@ describe('compiler compliance: styling', () => { type: MyComponent, selectors:[["my-component"]], decls: 1, - vars: 4, + vars: 5, consts: [[${AttributeMarker.Classes}, "grape"]], template: function MyComponent_Template(rf, $ctx$) { if (rf & 1) { @@ -863,8 +863,8 @@ describe('compiler compliance: styling', () => { } if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 2, $ctx$.myStyleExp)); - $r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 4, $ctx$.myClassExp)); + $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind1(1, 4, $ctx$.myStyleExp)); + $r3$.ɵɵclassMap($r3$.ɵɵpipeBind1(2, 6, $ctx$.myClassExp)); } } `; @@ -916,11 +916,11 @@ describe('compiler compliance: styling', () => { } if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); - $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 6, $ctx$.myStyleExp, 1000)); + $r3$.ɵɵstyleMap($r3$.ɵɵpipeBind2(1, 8, $ctx$.myStyleExp, 1000)); $r3$.ɵɵclassMap($e2_styling$); - $r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 9, $ctx$.barExp, 3000)); - $r3$.ɵɵstyleProp("baz", $r3$.ɵɵpipeBind2(3, 12, $ctx$.bazExp, 4000)); - $r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 15, $ctx$.fooExp, 2000)); + $r3$.ɵɵstyleProp("bar", $r3$.ɵɵpipeBind2(2, 11, $ctx$.barExp, 3000)); + $r3$.ɵɵstyleProp("baz", $r3$.ɵɵpipeBind2(3, 14, $ctx$.bazExp, 4000)); + $r3$.ɵɵclassProp("foo", $r3$.ɵɵpipeBind2(4, 17, $ctx$.fooExp, 2000)); $r3$.ɵɵadvance(5); $r3$.ɵɵtextInterpolate1(" ", $ctx$.item, ""); } @@ -1018,7 +1018,7 @@ describe('compiler compliance: styling', () => { const template = ` hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵɵallocHostVars(4); + $r3$.ɵɵallocHostVars(6); $r3$.ɵɵelementHostAttrs($e0_attrs$); } if (rf & 2) { @@ -1077,7 +1077,7 @@ describe('compiler compliance: styling', () => { const template = ` hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵɵallocHostVars(6); + $r3$.ɵɵallocHostVars(8); } if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); @@ -1152,7 +1152,7 @@ describe('compiler compliance: styling', () => { const hostBindings = ` hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵɵallocHostVars(4); + $r3$.ɵɵallocHostVars(6); } if (rf & 2) { $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); @@ -1218,7 +1218,7 @@ describe('compiler compliance: styling', () => { const template = ` function ClassDirective_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵɵallocHostVars(1); + $r3$.ɵɵallocHostVars(2); } if (rf & 2) { $r3$.ɵɵclassMap(ctx.myClassMap); @@ -1506,7 +1506,7 @@ describe('compiler compliance: styling', () => { … hostBindings: function MyComponent_HostBindings(rf, ctx, elIndex) { if (rf & 1) { - $r3$.ɵɵallocHostVars(4); + $r3$.ɵɵallocHostVars(6); $r3$.ɵɵelementHostAttrs($_c0$); } if (rf & 2) { diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index 4973975c5d54a..a3fda568d1806 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -588,5 +588,7 @@ function isHostStyling(): boolean { function getAndIncrementBindingIndex(lView: LView, isMapBased: boolean): number { // map-based bindings use two slots because the previously constructed // className / style value must be compared against. - return lView[BINDING_INDEX] += isMapBased ? 2 : 1; + const increment = isMapBased ? 2 : 1; + const index = lView[BINDING_INDEX] += increment; + return index - increment; } diff --git a/packages/core/src/render3/styling/bindings.ts b/packages/core/src/render3/styling/bindings.ts index 1cfba40ff7c27..a1f9552ce2fa8 100644 --- a/packages/core/src/render3/styling/bindings.ts +++ b/packages/core/src/render3/styling/bindings.ts @@ -10,11 +10,10 @@ import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanit import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer'; import {ApplyStylingFn, LStylingData, StylingMapArray, StylingMapArrayIndex, StylingMapsSyncMode, SyncStylingMapsFn, TStylingConfig, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from '../interfaces/styling'; import {NO_CHANGE} from '../tokens'; -import {DEFAULT_BINDING_INDEX, DEFAULT_BINDING_VALUE, DEFAULT_GUARD_MASK_VALUE, MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, forceStylesAsString, getBindingValue, getConfig, getDefaultValue, getGuardMask, getInitialStylingValue, getMapProp, getMapValue, getProp, getPropValuesStartPosition, getStylingMapArray, getTotalSources, getValue, getValuesCount, hasConfig, hasValueChanged, isContextLocked, isHostStylingActive, isSanitizationRequired, isStylingValueDefined, lockContext, normalizeIntoStylingMap, patchConfig, setDefaultValue, setGuardMask, setMapAsDirty, setValue} from '../util/styling_utils'; - +import {DEFAULT_BINDING_INDEX, DEFAULT_BINDING_VALUE, DEFAULT_GUARD_MASK_VALUE, MAP_BASED_ENTRY_PROP_NAME, TEMPLATE_DIRECTIVE_INDEX, concatString, forceStylesAsString, getBindingValue, getConfig, getDefaultValue, getGuardMask, getInitialStylingValue, getMapProp, getMapValue, getProp, getPropValuesStartPosition, getStylingMapArray, getTotalSources, getValue, getValuesCount, hasConfig, hasValueChanged, isContextLocked, isHostStylingActive, isSanitizationRequired, isStylingMapArray, isStylingValueDefined, lockContext, normalizeIntoStylingMap, patchConfig, setDefaultValue, setGuardMask, setMapAsDirty, setValue} from '../util/styling_utils'; import {getStylingState, resetStylingState} from './state'; - +const VALUE_IS_EXTERNALLY_MODIFIED = {}; /** * -------- @@ -658,36 +657,65 @@ export function applyStylingMapDirectly( bindingIndex: number, value: {[key: string]: any} | string | null, isClassBased: boolean, sanitizer?: StyleSanitizeFn | null, forceUpdate?: boolean, bindingValueContainsInitial?: boolean): void { - const config = getConfig(context); - const writeToAttrDirectly = !(config & TStylingConfig.HasPropBindings); const oldValue = getValue(data, bindingIndex); - if (!writeToAttrDirectly && value !== NO_CHANGE) { - value = normalizeIntoStylingMap(oldValue, value, !isClassBased); - } - if (forceUpdate || hasValueChanged(oldValue, value)) { + const config = getConfig(context); const hasInitial = config & TStylingConfig.HasInitialStyling; + const initialValue = + hasInitial && !bindingValueContainsInitial ? getInitialStylingValue(context) : null; setValue(data, bindingIndex, value); + // the cached value is the last snapshot of the style or class + // attribute value and is used in the if statement below to + // keep track of internal/external changes. + const cachedValueIndex = bindingIndex + 1; + let cachedValue = getValue(data, cachedValueIndex); + if (cachedValue === NO_CHANGE) { + cachedValue = initialValue; + } + cachedValue = typeof cachedValue !== 'string' ? '' : cachedValue; + + // If a class/style value was modified externally then the styling + // fast pass cannot guarantee that the external values are retained. + // When this happens, the algorithm will bail out and not write to + // the style or className attribute directly. + let writeToAttrDirectly = !(config & TStylingConfig.HasPropBindings); + if (writeToAttrDirectly && + checkIfExternallyModified(element as HTMLElement, cachedValue, isClassBased)) { + writeToAttrDirectly = false; + if (oldValue !== VALUE_IS_EXTERNALLY_MODIFIED) { + // direct styling will reset the attribute entirely each time, + // and, for this reason, if the algorithm decides it cannot + // write to the class/style attributes directly then it must + // reset all the previous style/class values before it starts + // to apply values in the non-direct way. + removeStylingValues(renderer, element, oldValue, isClassBased); + + // this will instruct the algorithm not to apply class or style + // values directly anymore. + setValue(data, cachedValueIndex, VALUE_IS_EXTERNALLY_MODIFIED); + } + } + if (writeToAttrDirectly) { - const initialValue = - hasInitial && !bindingValueContainsInitial ? getInitialStylingValue(context) : null; + let valueToApply: string; if (isClassBased) { - let valueToApply = typeof value === 'string' ? value : objectToClassName(value); + valueToApply = typeof value === 'string' ? value : objectToClassName(value); if (initialValue !== null) { - valueToApply = initialValue + ' ' + valueToApply; + valueToApply = concatString(initialValue, valueToApply, ' '); } setClassName(renderer, element, valueToApply); } else { - let valueToApply = forceStylesAsString(value as{[key: string]: any}, true); + valueToApply = forceStylesAsString(value as{[key: string]: any}, true); if (initialValue !== null) { valueToApply = initialValue + ';' + valueToApply; } setStyleAttr(renderer, element, valueToApply); } + setValue(data, cachedValueIndex, valueToApply || null); } else { const applyFn = isClassBased ? setClass : setStyle; - const map = value as StylingMapArray; + const map = normalizeIntoStylingMap(oldValue, value, !isClassBased); const initialStyles = hasInitial ? getStylingMapArray(context) : null; for (let i = StylingMapArrayIndex.ValuesStartPosition; i < map.length; @@ -974,3 +1002,52 @@ function objectToClassName(obj: {[key: string]: any} | null): string { } return str; } + +/** + * Determines whether or not an element style/className value has changed since the last update. + * + * This function helps Angular determine if a style or class attribute value was + * modified by an external plugin or API outside of the style binding code. This + * means any JS code that adds/removes class/style values on an element outside + * of Angular's styling binding algorithm. + * + * @returns true when the value was modified externally. + */ +function checkIfExternallyModified(element: RElement, cachedValue: any, isClassBased: boolean) { + // this means it was checked before and there is no reason + // to compare the style/class values again. + if (cachedValue === VALUE_IS_EXTERNALLY_MODIFIED) return true; + + // web workers are not covered in this case + if (typeof Element === 'undefined' || !(element instanceof Element)) return false; + + // comparing the DOM value against the cached value is the best way to + // see if something has changed. + const e = element as HTMLElement; + const currentValue = isClassBased ? e.className : (e.style && e.style.cssText); + return currentValue !== (cachedValue || ''); +} + +/** + * Removes provided styling values from the element + */ +function removeStylingValues( + renderer: any, element: RElement, values: string | {[key: string]: any} | StylingMapArray, + isClassBased: boolean) { + let arr: StylingMapArray; + if (isStylingMapArray(values)) { + arr = values as StylingMapArray; + } else { + arr = normalizeIntoStylingMap(null, values, !isClassBased); + } + + const applyFn = isClassBased ? setClass : setStyle; + for (let i = StylingMapArrayIndex.ValuesStartPosition; i < arr.length; + i += StylingMapArrayIndex.TupleSize) { + const value = getMapValue(arr, i); + if (value) { + const prop = getMapProp(arr, i); + applyFn(renderer, element, prop, false); + } + } +} diff --git a/packages/core/src/render3/styling/styling_debug.ts b/packages/core/src/render3/styling/styling_debug.ts index 981f46d6218f9..1369d411b750b 100644 --- a/packages/core/src/render3/styling/styling_debug.ts +++ b/packages/core/src/render3/styling/styling_debug.ts @@ -430,7 +430,20 @@ export class NodeStylingDebug implements DebugNodeStyling { */ get values(): {[key: string]: any} { const entries: {[key: string]: any} = {}; - this._mapValues(this._data, (prop: string, value: any) => { entries[prop] = value; }); + const config = this.config; + let data = this._data; + + // the direct pass code doesn't convert [style] or [class] values + // into StylingMapArray instances. For this reason, the values + // need to be converted ahead of time since the styling debug + // relies on context resolution to figure out what styling + // values have been added/removed on the element. + if (config.allowDirectStyling && config.hasMapBindings) { + data = data.concat([]); // make a copy + this._convertMapBindingsToStylingMapArrays(data); + } + + this._mapValues(data, (prop: string, value: any) => { entries[prop] = value; }); return entries; } diff --git a/packages/core/src/render3/util/styling_utils.ts b/packages/core/src/render3/util/styling_utils.ts index d846512435da2..1d6e3c9bcb353 100644 --- a/packages/core/src/render3/util/styling_utils.ts +++ b/packages/core/src/render3/util/styling_utils.ts @@ -247,7 +247,7 @@ export function isStylingContext(value: any): boolean { typeof value[1] !== 'string'; } -export function isStylingMapArray(value: TStylingContext | StylingMapArray | null): boolean { +export function isStylingMapArray(value: any): boolean { // the StylingMapArray is in the format of [initial, prop, string, prop, string] // and this is the defining value to distinguish between arrays return Array.isArray(value) && @@ -303,7 +303,10 @@ export function forceStylesAsString( for (let i = 0; i < props.length; i++) { const prop = props[i]; const propLabel = hyphenateProps ? hyphenate(prop) : prop; - str = concatString(str, `${propLabel}:${styles[prop]}`, ';'); + const value = styles[prop]; + if (value !== null) { + str = concatString(str, `${propLabel}:${value}`, ';'); + } } } return str; diff --git a/packages/core/test/acceptance/i18n_spec.ts b/packages/core/test/acceptance/i18n_spec.ts index 344384e02264d..16fd89ffd5f29 100644 --- a/packages/core/test/acceptance/i18n_spec.ts +++ b/packages/core/test/acceptance/i18n_spec.ts @@ -1268,7 +1268,7 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) .toEqual( - `
traduction: un email ` + + `
traduction: un email ` + `
`); directiveInstances.forEach(instance => instance.klass = 'bar'); @@ -1277,7 +1277,7 @@ onlyInIvy('Ivy i18n logic').describe('runtime i18n', () => { fixture.detectChanges(); expect(fixture.nativeElement.innerHTML) .toEqual( - `
traduction: 2 emails ` + + `
traduction: 2 emails ` + `
`); }); diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index 25a4f19a7fa99..faf82f32a5321 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -2309,6 +2309,34 @@ describe('styling', () => { expect(fixture.debugElement.nativeElement.innerHTML).toContain('two'); expect(fixture.debugElement.nativeElement.innerHTML).toContain('three'); }); + + onlyInIvy('only ivy treats [class] in concert with other class bindings') + .it('should retain classes added externally', () => { + @Component({template: `
`}) + class MyComp { + exp = ''; + } + + const fixture = + TestBed.configureTestingModule({declarations: [MyComp]}).createComponent(MyComp); + fixture.detectChanges(); + + const div = fixture.nativeElement.querySelector('div') !; + div.className += ' abc'; + expect(splitSortJoin(div.className)).toEqual('abc'); + + fixture.componentInstance.exp = '1 2 3'; + fixture.detectChanges(); + + expect(splitSortJoin(div.className)).toEqual('1 2 3 abc'); + + fixture.componentInstance.exp = '4 5 6 7'; + fixture.detectChanges(); + + expect(splitSortJoin(div.className)).toEqual('4 5 6 7 abc'); + + function splitSortJoin(s: string) { return s.split(/\s+/).sort().join(' ').trim(); } + }); }); function assertStyleCounters(countForSet: number, countForRemove: number) { diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 02ba8c6a28b80..b9743ff592f93 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -596,6 +596,9 @@ { "name": "getActiveDirectiveId" }, + { + "name": "getAndIncrementBindingIndex" + }, { "name": "getBeforeNodeForView" },