fixup! feat(ivy): properly apply style="", [style], [style.foo] and […

…attr.style] bindings

packages/compiler/src/render3/view/template.ts

@@ -307,14 +307,15 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
     // Add the attributes
     const i18nMessages: o.Statement[] = [];
     const attributes: o.Expression[] = [];
-    const initialStyles: {key: string, quoted: boolean, value: o.Expression}[] = [];
+    const initialStyles: o.Expression[] = [];
 
     Object.getOwnPropertyNames(outputAttrs).forEach(name => {
       const value = outputAttrs[name];
       if (name == 'style') {
         const styles = parseStyle(value);
         Object.keys(styles).forEach(key => {
-          initialStyles.push({key, value: o.literal(styles[key]), quoted: key.indexOf('-') >= 0});
+          initialStyles.push(o.literal(key));
+          initialStyles.push(o.literal(styles[key]));
         });
       } else {
         attributes.push(o.literal(name));
@@ -364,8 +365,6 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
       this.addNamespaceInstruction(currentNamespace, element);
     }
 
-    const isEmptyElement = element.children.length === 0 && element.outputs.length === 0;
-
     const implicit = o.variable(CONTEXT_NAME);
 
     const styleInputs: t.BoundAttribute[] = [];
@@ -387,7 +386,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
 
     const elementStyleIndex =
         (initialStyles.length || styleInputs.length) ? this.allocateDataSlot() : 0;
-    const createSelfClosingInstruction = elementStyleIndex === 0 && isEmptyElement;
+    const createSelfClosingInstruction =
+        elementStyleIndex === 0 && element.children.length === 0 && element.outputs.length === 0;
 
     if (createSelfClosingInstruction) {
       this.instruction(
@@ -403,9 +403,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
 
       // initial styling for static style="..." attributes
       if (elementStyleIndex > 0) {
-        let paramsList: (o.LiteralExpr | o.LiteralMapExpr)[] = [o.literal(elementStyleIndex)];
-        if (Object.keys(initialStyles).length) {
-          paramsList.push(o.literalMap(initialStyles));
+        let paramsList: (o.Expression)[] = [o.literal(elementStyleIndex)];
+        if (initialStyles.length) {
+          // the template compiler handles initial styling (e.g. style="foo") values
+          // in a special command called `elementStyle` so that the initial styles
+          // can be processed during runtime. These initial styles values are bound to
+          // a constant because the inital style values do not change (since they're static).
+          paramsList.push(this.constantPool.getConstLiteral(o.literalArr(initialStyles), true));
         }
         this._creationCode.push(o.importExpr(R3.elementStyling).callFn(paramsList).toStmt());
       }

packages/compiler/test/render3/r3_view_compiler_styling_spec.ts

@@ -83,21 +83,30 @@ describe('compiler compliance: styling', () => {
          };
 
          const template = `
-          template: function MyComponent_Template(rf: $RenderFlags$, $ctx$: $MyComponent$) {
-            if (rf & 1) {
-              $r3$.ɵE(0, 'div');
-              $r3$.ɵs(1, { opacity: '1' });
-              $r3$.ɵe();
-            }
-            if (rf & 2) {
-              $r3$.ɵsm(1, $ctx$.myStyleExp);
-              $r3$.ɵsp(1, 'width', $ctx$.myWidth);
-              $r3$.ɵsp(1, 'height', $ctx$.myHeight);
-              $r3$.ɵsa(1);
-              $r3$.ɵa(0, 'style', $r3$.ɵb('border-width: 10px'));
-            }
-          }
-          `;
+          const _c0 = ['opacity','1'];
+          class MyComponent {
+            static ngComponentDef = i0.ɵdefineComponent({
+              type: MyComponent,
+              selectors:[['my-component']],
+              factory:function MyComponent_Factory(){
+                return new MyComponent();
+              },
+              template: function MyComponent_Template(rf: $RenderFlags$, $ctx$: $MyComponent$) {
+                if (rf & 1) {
+                  $r3$.ɵE(0, 'div');
+                  $r3$.ɵs(1, _c0);
+                  $r3$.ɵe();
+                }
+                if (rf & 2) {
+                  $r3$.ɵsm(1, $ctx$.myStyleExp);
+                  $r3$.ɵsp(1, 'width', $ctx$.myWidth);
+                  $r3$.ɵsp(1, 'height', $ctx$.myHeight);
+                  $r3$.ɵsa(1);
+                  $r3$.ɵa(0, 'style', $r3$.ɵb('border-width: 10px'));
+                }
+              }
+            });
+        `;
 
          const result = compile(files, angularFiles);
          expectEmit(result.source, template, 'Incorrect template');

packages/core/src/render3/instructions.ts

@@ -1282,10 +1282,19 @@ export function elementClass<T>(index: number, value: T | NO_CHANGE): void {
  *   bindings. If a style binding changes its value to null then the initial styling
  *   values that are passed in here will be applied to the element (if matched).
  */
-export function elementStyling<T>(index: number, styles?: {[key: string]: any}): void {
-  const hasData = styles && Object.keys(styles).length > 0;
+export function elementStyling<T>(index: number, styles?: string[]): void {
+  const hasData = styles && styles.length;
+  let initialStyles: {[key: string]: any}|null = null;
+  if (hasData) {
+    initialStyles = {};
+    for (let i = 0; i < styles !.length; i += 2) {
+      const prop = styles ![i];
+      const value = styles ![i + 1];
+      initialStyles[prop] = value;
+    }
+  }
   viewData[index + HEADER_OFFSET] =
-      {initial: styles || null, multi: null, single: null, updated: hasData};
+      {initial: initialStyles, multi: null, single: null, updated: hasData};
   if (hasData) {
     elementStylingApply(index);
   }

packages/core/src/render3/styling.ts

@@ -8,16 +8,54 @@
 import {LElementNode} from './interfaces/node';
 import {ProceduralRenderer3, Renderer3, RendererStyleFlags3, isProceduralRenderer} from './interfaces/renderer';
 
+/**
+ * Given that Angular can apply styling using various methods: attrs, style and multi-prop bindings
+ * ...
+ * A record of styles needs to be tracked for each element that contains these properties.
+ *
+ * `ElementStyleRecord` is designed to do this.
+ */
 export interface ElementStyleRecord {
+  /**
+    * whether or not the record has been updated since
+    * the last time the styles were rendered.
+    */
   updated: boolean;
+
+  /**
+    * The initial styles that were applied to the element
+    * during creation (e.g. `<div style="initial: style;">`)
+    */
   initial: {[key: string]: any}|null;
+
+  /**
+    * All styles that were assigned using `updateElementStyleMap`
+    * (These are populated from `[style]` bindings.)
+    */
   multi: {[key: string]: any}|null;
+
+  /**
+    * All individual styles that were assigned using `updateElementStyleProp`
+    * (These are populated from `[style.prop]` bindings.)
+    */
   single: {[key: string]: any}|null;
 }
 
+/**
+ * Sets and resolves all `multi` styles on an `ElementStyleRecord` so that they can be
+ * applied to the element once `renderElementStyles` is called.
+ *
+ * All missing styles (any values that are not provided in the new `styles` param)
+ * will be set to null in the `multi` param within `ElementStyleRecord`.
+ *
+ * @param record The styling record that will be updated with the
+ *    newly provided style values.
+ * @param styles The key/value map of CSS styles that will be used for the update.
+ */
 export function updateElementStyleMap(
     record: ElementStyleRecord, styles: {[key: string]: any} | null): void {
   const currentStyles = record.multi || {};
+
   if (styles) {
     const allProps = Object.keys(styles).concat(Object.keys(currentStyles));
     for (let i = 0; i < allProps.length; i++) {
@@ -44,6 +82,19 @@ export function updateElementStyleMap(
   record.multi = currentStyles;
 }
 
+/**
+ * Sets and resolves a single CSS style on a property on an `ElementStyleRecord` so that they
+ * can be applied to the element once `renderElementStyles` is called.
+ *
+ * Note that prop-level styles are considered higher priority than styles that are applied
+ * using `updateElementStyleMap`, therefore, when styles are rendered then any styles that
+ * have been applied using this function will be considered first.
+ *
+ * @param record The styling record that will be updated with the
+ *    newly provided style value.
+ * @param prop The CSS style prop
+ * @param valueToAdd The CSS style value that will be assigned
+ */
 export function updateElementStyleProp(
     record: ElementStyleRecord, prop: string, valueToAdd: string | null): void {
   const valueToCompare = (record.single = record.single || {})[prop];
@@ -53,9 +104,23 @@ export function updateElementStyleProp(
   }
 }
 
+/**
+ * Renders all queued styles using a renderer onto the given element.
+ *
+ * This function works by rendering any styles (that have been applied
+ * using `updateElementStyleMap` and `updateElementStyleProp`) onto the
+ * provided element using the provided renderer. Just before the styles
+ * are rendered a final key/value style map will be assembled.
+ *
+ * @param lElement the element that the styles will be rendered on
+ * @param record The styling record that will be updated with the
+ *    newly provided style value.
+ * @param renderer the renderer that will be used to apply the styling
+ * @returns an object literal. `{ color: 'red', height: 'auto'}`.
+ */
 export function renderElementStyles(
-    lElement: LElementNode, record: ElementStyleRecord, renderer: Renderer3) {
-  if (!record.updated) return null;
+    lElement: LElementNode, record: ElementStyleRecord, renderer: Renderer3): void {
+  if (!record.updated) return;
 
   const stylesToApply = buildElementStyles(record);
   if (!stylesToApply) return;
@@ -86,6 +151,26 @@ export function renderElementStyles(
   record.updated = false;
 }
 
+/**
+ * Constructs a final key/value object literal of styles based on a combination of the
+ * initial, multi and single-property styles (all of which are housed within
+ * the `ElementStyleRecord` value).
+ *
+ * The goal of the function below is to construct a single key/value object
+ * containing the styles. The process for the assembling of styles work as
+ * such:
+ *
+ * 1. First loop over all the single-property styles and build an object literal using them.
+ * 2. Then loop over the multi styles and see which ones are missing or null
+ *    in the constructed object literal. If so then add them.
+ * 3. Finally loop over the initial styles and see which ones are missing or null
+ *    in the constructed object literal. If so then add them.
+ * 4. Return the final object literal.
+ *
+ * @param record The styling record that will be updated with the
+ *    newly provided style value.
+ * @returns the object literal `{key: "value"}`.
+ */
 export function buildElementStyles(record: ElementStyleRecord): {[key: string]: any}|null {
   const styles: {[key: string]: any} = record.single ? {...record.single} : {};
   if (record.multi) {

packages/core/test/render3/compiler_canonical/elements_spec.ts

@@ -367,7 +367,7 @@ describe('elements', () => {
           template: function MyComponent_Template(rf: $RenderFlags$, ctx: $MyComponent$) {
             if (rf & 1) {
               $r3$.ɵE(0, 'div');
-              $r3$.ɵs(1, {color: 'red'});
+              $r3$.ɵs(1, ['color', 'red']);
               $r3$.ɵe();
             }
             if (rf & 2) {

packages/core/test/render3/instructions_spec.ts

@@ -22,13 +22,13 @@ import {ComponentFixture, TemplateFixture} from './render_util';
 describe('instructions', () => {
   function createAnchor() {
     elementStart(0, 'a');
-    elementStyling(1, {});
+    elementStyling(1);
     elementEnd();
   }
 
   function createDiv() {
     elementStart(0, 'div');
-    elementStyling(1, {});
+    elementStyling(1);
     elementEnd();
   }
 
@@ -212,7 +212,7 @@ describe('instructions', () => {
   describe('elementStyleMap', () => {
     function createDivWithStyle() {
       elementStart(0, 'div');
-      elementStyling(1, {height: '10px'});
+      elementStyling(1, ['height', '10px']);
       elementEnd();
     }
 

packages/core/test/render3/integration_spec.ts

@@ -744,7 +744,7 @@ describe('render3 integration test', () => {
         function Template(rf: RenderFlags, ctx: any) {
           if (rf & RenderFlags.Create) {
             elementStart(0, 'span');
-            elementStyling(1, {});
+            elementStyling(1);
             elementEnd();
           }
           if (rf & RenderFlags.Update) {
@@ -763,7 +763,7 @@ describe('render3 integration test', () => {
         function Template(rf: RenderFlags, ctx: any) {
           if (rf & RenderFlags.Create) {
             elementStart(0, 'span');
-            elementStyling(1, {});
+            elementStyling(1);
             elementEnd();
           }
           if (rf & RenderFlags.Update) {

packages/core/test/render3/styling_spec.ts

@@ -19,7 +19,7 @@ describe('styling', () => {
          function Template(rf: RenderFlags, ctx: any) {
            if (rf & RenderFlags.Create) {
              elementStart(0, 'span');
-             elementStyling(1, {width: '100px', height: '100px', opacity: 0.5});
+             elementStyling(1, ['width', '100px', 'height', '100px', 'opacity', '0.5']);
              elementEnd();
            }
            if (rf & RenderFlags.Update) {