feat(ivy): convert [ngStyle] and [ngClass] to use ivy styling bindings (#28711)

Prior to this fix, both the `NgStyle` and `NgClass` directives made use
of `Renderer2` and this dependency raised issues for future versions of
Angular that cannot inject it. This patch ensures that there are two
versions of both directives: one for the VE and another for Ivy.

Jira Issue: FW-882

PR Close #28711
This commit is contained in:
Matias Niemelä
2019-02-18 16:12:42 -08:00
committed by Igor Minar
parent d0e81eb593
commit cfb2d176f8
15 changed files with 921 additions and 228 deletions

View File

@ -105,7 +105,7 @@ export {
export {NgModuleFactory, NgModuleRef, NgModuleType} from './ng_module_ref';
export {
AttributeMarker
AttributeMarker
} from './interfaces/node';
export {

View File

@ -6,7 +6,6 @@
* found in the LICENSE file at https://angular.io/license
*/
import {StyleSanitizeFn} from '../../sanitization/style_sanitizer';
import {assertNotEqual} from '../../util/assert';
import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty';
import {AttributeMarker, TAttributes} from '../interfaces/node';
import {BindingStore, BindingType, Player, PlayerBuilder, PlayerFactory, PlayerIndex} from '../interfaces/player';
@ -418,16 +417,9 @@ export function updateContextWithBindings(
const directiveMultiStylesStartIndex =
multiStylesStartIndex + totalCurrentStyleBindings * StylingIndex.Size;
const cachedStyleMapIndex = cachedStyleMapValues.length;
// this means that ONLY directive style styling (like ngStyle) was used
// therefore the root directive will still need to be filled in
if (directiveIndex > 0 &&
cachedStyleMapValues.length <= MapBasedOffsetValuesIndex.ValuesStartPosition) {
cachedStyleMapValues.push(0, directiveMultiStylesStartIndex, null, 0);
}
cachedStyleMapValues.push(
0, directiveMultiStylesStartIndex, null, filteredStyleBindingNames.length);
registerMultiMapEntry(
context, directiveIndex, false, directiveMultiStylesStartIndex,
filteredStyleBindingNames.length);
for (let i = MapBasedOffsetValuesIndex.ValuesStartPosition; i < cachedStyleMapIndex;
i += MapBasedOffsetValuesIndex.Size) {
@ -441,16 +433,9 @@ export function updateContextWithBindings(
const directiveMultiClassesStartIndex =
multiClassesStartIndex + totalCurrentClassBindings * StylingIndex.Size;
const cachedClassMapIndex = cachedClassMapValues.length;
// this means that ONLY directive class styling (like ngClass) was used
// therefore the root directive will still need to be filled in
if (directiveIndex > 0 &&
cachedClassMapValues.length <= MapBasedOffsetValuesIndex.ValuesStartPosition) {
cachedClassMapValues.push(0, directiveMultiClassesStartIndex, null, 0);
}
cachedClassMapValues.push(
0, directiveMultiClassesStartIndex, null, filteredClassBindingNames.length);
registerMultiMapEntry(
context, directiveIndex, true, directiveMultiClassesStartIndex,
filteredClassBindingNames.length);
for (let i = MapBasedOffsetValuesIndex.ValuesStartPosition; i < cachedClassMapIndex;
i += MapBasedOffsetValuesIndex.Size) {
@ -619,7 +604,7 @@ export function updateStylingMap(
}
const multiStylesStartIndex = getMultiStylesStartIndex(context);
let multiClassesStartIndex = getMultiClassStartIndex(context);
let multiClassesStartIndex = getMultiClassesStartIndex(context);
let multiClassesEndIndex = context.length;
if (!ignoreAllStyleUpdates) {
@ -862,6 +847,7 @@ function patchStylingMapIntoContext(
valuesEntryShapeChange = true; // some values are missing
const ctxValue = getValue(context, ctxIndex);
const ctxFlag = getPointers(context, ctxIndex);
const ctxDirective = getDirectiveIndexFromEntry(context, ctxIndex);
if (ctxValue != null) {
valuesEntryShapeChange = true;
}
@ -1293,7 +1279,7 @@ function getMultiStartIndex(context: StylingContext): number {
return getMultiOrSingleIndex(context[StylingIndex.MasterFlagPosition]) as number;
}
function getMultiClassStartIndex(context: StylingContext): number {
function getMultiClassesStartIndex(context: StylingContext): number {
const classCache = context[StylingIndex.CachedMultiClasses];
return classCache
[MapBasedOffsetValuesIndex.ValuesStartPosition +
@ -1625,15 +1611,31 @@ export function getDirectiveIndexFromEntry(context: StylingContext, index: numbe
return value & DirectiveOwnerAndPlayerBuilderIndex.BitMask;
}
function getDirectiveIndexFromRegistry(context: StylingContext, directive: any) {
const index =
getDirectiveRegistryValuesIndexOf(context[StylingIndex.DirectiveRegistryPosition], directive);
ngDevMode &&
assertNotEqual(
index, -1,
`The provided directive ${directive} has not been allocated to the element\'s style/class bindings`);
return index > 0 ? index / DirectiveRegistryValuesIndex.Size : 0;
// return index / DirectiveRegistryValuesIndex.Size;
function getDirectiveIndexFromRegistry(context: StylingContext, directiveRef: any) {
let directiveIndex: number;
const dirs = context[StylingIndex.DirectiveRegistryPosition];
let index = getDirectiveRegistryValuesIndexOf(dirs, directiveRef);
if (index === -1) {
// if the directive was not allocated then this means that styling is
// being applied in a dynamic way AFTER the element was already instantiated
index = dirs.length;
directiveIndex = index > 0 ? index / DirectiveRegistryValuesIndex.Size : 0;
dirs.push(null, null, null, null);
dirs[index + DirectiveRegistryValuesIndex.DirectiveValueOffset] = directiveRef;
dirs[index + DirectiveRegistryValuesIndex.DirtyFlagOffset] = false;
dirs[index + DirectiveRegistryValuesIndex.SinglePropValuesIndexOffset] = -1;
const classesStartIndex =
getMultiClassesStartIndex(context) || StylingIndex.SingleStylesStartPosition;
registerMultiMapEntry(context, directiveIndex, true, context.length);
registerMultiMapEntry(context, directiveIndex, false, classesStartIndex);
} else {
directiveIndex = index > 0 ? index / DirectiveRegistryValuesIndex.Size : 0;
}
return directiveIndex;
}
function getDirectiveRegistryValuesIndexOf(
@ -1936,3 +1938,21 @@ function hyphenate(value: string): string {
return value.replace(
/[a-z][A-Z]/g, match => `${match.charAt(0)}-${match.charAt(1).toLowerCase()}`);
}
function registerMultiMapEntry(
context: StylingContext, directiveIndex: number, entryIsClassBased: boolean,
startPosition: number, count = 0) {
const cachedValues =
context[entryIsClassBased ? StylingIndex.CachedMultiClasses : StylingIndex.CachedMultiStyles];
if (directiveIndex > 0) {
const limit = MapBasedOffsetValuesIndex.ValuesStartPosition +
(directiveIndex * MapBasedOffsetValuesIndex.Size);
while (cachedValues.length < limit) {
// this means that ONLY directive class styling (like ngClass) was used
// therefore the root directive will still need to be filled in as well
// as any other directive spaces incase they only used static values
cachedValues.push(0, startPosition, null, 0);
}
}
cachedValues.push(0, startPosition, null, count);
}

View File

@ -56,6 +56,16 @@ export function allocStylingContext(
element: RElement | null, templateStyleContext: StylingContext): StylingContext {
// each instance gets a copy
const context = templateStyleContext.slice() as any as StylingContext;
// the HEADER values contain arrays which also need
// to be copied over into the new context
for (let i = 0; i < StylingIndex.SingleStylesStartPosition; i++) {
const value = templateStyleContext[i];
if (Array.isArray(value)) {
context[i] = value.slice();
}
}
context[StylingIndex.ElementPosition] = element;
// this will prevent any other directives from extending the context

View File

@ -91,7 +91,7 @@ class BoxWithOverriddenStylesComponent {
<box-with-overridden-styles
style="display:block"
[style]="{'border-radius':'50px', 'border': '50px solid teal'}">
[style]="{'border-radius':'50px', 'border': '50px solid teal'}" [ngStyle]="{transform:'rotate(50deg)'}">
</box-with-overridden-styles>
`,
})

View File

@ -722,6 +722,9 @@
{
"name": "getMatchingBindingIndex"
},
{
"name": "getMultiClassesStartIndex"
},
{
"name": "getMultiOrSingleIndex"
},
@ -1100,6 +1103,9 @@
{
"name": "refreshDynamicEmbeddedViews"
},
{
"name": "registerMultiMapEntry"
},
{
"name": "registerPostOrderHooks"
},

View File

@ -1693,18 +1693,6 @@ describe('style and class based bindings', () => {
]);
});
it('should throw an error if a directive is provided that isn\'t registered', () => {
const template = createEmptyStylingContext();
const knownDir = {};
const unknownDir = {};
updateContextWithBindings(template, knownDir, null, ['color']);
const ctx = allocStylingContext(element, template);
updateStyleProp(ctx, 0, 'blue', knownDir);
expect(() => { updateStyleProp(ctx, 0, 'blue', unknownDir); }).toThrow();
});
it('should use a different sanitizer when a different directive\'s binding is updated',
() => {
const getStyles = trackStylesFactory();
@ -1766,6 +1754,62 @@ describe('style and class based bindings', () => {
expect(((ctx[colorIndex] as number) & StylingFlags.Sanitize) > 0).toBeFalsy();
expect(getStyles(ctx, dirWithoutSanitizer)).toEqual({color: 'green'});
});
it('should automatically register a styling context with a foreign directive if styling is applied with said directive',
() => {
const template = createEmptyStylingContext();
const knownDir = {};
const foreignDir = {};
updateContextWithBindings(template, knownDir);
const ctx = allocStylingContext(element, template);
expect(ctx[StylingIndex.DirectiveRegistryPosition]).toEqual([
null, //
-1, //
false, //
null, //
knownDir, //
2, //
false, //
null, //
]);
expect(ctx[StylingIndex.CachedMultiClasses].length)
.toEqual(template[StylingIndex.CachedMultiClasses].length);
expect(ctx[StylingIndex.CachedMultiClasses]).toEqual([0, 0, 9, null, 0, 0, 9, null, 0]);
expect(ctx[StylingIndex.CachedMultiStyles].length)
.toEqual(template[StylingIndex.CachedMultiStyles].length);
expect(ctx[StylingIndex.CachedMultiStyles]).toEqual([0, 0, 9, null, 0, 0, 9, null, 0]);
updateStylingMap(ctx, 'foo', null, foreignDir);
expect(ctx[StylingIndex.DirectiveRegistryPosition]).toEqual([
null, //
-1, //
false, //
null, //
knownDir, //
2, //
false, //
null, //
foreignDir, //
-1, //
true, //
null, //
]);
expect(ctx[StylingIndex.CachedMultiClasses].length)
.not.toEqual(template[StylingIndex.CachedMultiClasses].length);
expect(ctx[StylingIndex.CachedMultiClasses]).toEqual([
1, 0, 9, null, 0, 0, 9, null, 0, 0, 9, 'foo', 1
]);
expect(ctx[StylingIndex.CachedMultiStyles].length)
.not.toEqual(template[StylingIndex.CachedMultiStyles].length);
expect(ctx[StylingIndex.CachedMultiStyles]).toEqual([
0, 0, 9, null, 0, 0, 9, null, 0, 0, 9, null, 0
]);
});
});
it('should skip issuing style updates if there is nothing to update upon first render', () => {