feat: Simplify rewrite for dynamic attribute values.

chriseppstein · chriseppstein · commit 0717e9365095 · 2020-08-09T10:58:05.000-07:00
This change greatly simplifies the per-element rewrite for elements that
have dynamic attribute values. In the element rewrite we no longer
enumerate all the attribute values as possible styles and we don't
enumerate the specific values as part of the switch condition. Instead
we just reference the attribute as being a switch condition and let
the runtime fill in the possible values from new information that is
passed in for the aggregate rewrite data.

The aggregate rewrite data is more explicit than it strictly needs to
be, the rewrite code could do some sort of string manipulation in order
to construct the attribute value style name instead of looking it up in
a table. However that solution would preclude a later optimization where
we optimize away the strings and replace them with unique numbers.

In production code this change reduces the rewrite significantly (by
more than 30 arguments in one element I examined) and should produce a
significant savings in aggregate by avoiding duplication when the same
attributes are used dynamically on different template elements
throughout the application.
diff --git a/packages/@css-blocks/ember-app/runtime/app/services/StyleEvaluator.ts b/packages/@css-blocks/ember-app/runtime/app/services/StyleEvaluator.ts
@@ -1,3 +1,5 @@
+import type { ObjectDictionary } from "@opticss/util";
+
 import { AggregateRewriteData } from "./AggregateRewriteData";
 
 export type ClassNameExpression = Array<string | number | boolean | null>;
@@ -41,6 +43,7 @@ export class StyleEvaluator {
   // because the block it implements has changed since the implementing block
   // was precompiled and released.
   blockStyleIds: Array<Array<number | null>> = [];
+  blockGroups: Array<ObjectDictionary<ObjectDictionary<string>>> = [];
   styles: Array<number | null> = [];
   constructor(data: AggregateRewriteData, args: ClassNameExpression) {
     this.data = data;
@@ -60,6 +63,7 @@ export class StyleEvaluator {
       let styleIds = blockInfo.implementations[runtimeBlockIndex];
       assert(styleIds, "unknown implementation");
       this.blockStyleIds.push(styleIds);
+      this.blockGroups.push(blockInfo.groups);
     }
   }
 
@@ -99,6 +103,7 @@ export class StyleEvaluator {
     // to true.
     let numConditions = this.num();
     let styleStates = new Array<boolean>(this.styles.length);
+    let stylesApplied = new Set<number>();
     while (numConditions--) {
       let condition = this.num();
       switch (condition) {
@@ -113,14 +118,13 @@ export class StyleEvaluator {
           this.evaluateTernaryCondition(styleStates);
           break;
         case Condition.switch:
-          this.evaluateSwitchCondition(styleStates);
+          this.evaluateSwitchCondition(stylesApplied);
           break;
         default:
           throw new Error(`Unknown condition type ${condition}`);
       }
     }
 
-    let stylesApplied = new Set<number>();
     for (let i = 0; i < this.styles.length; i++) {
       if (styleStates[i] && this.styles[i] !== null) {
         stylesApplied.add(this.styles[i]!);
@@ -129,33 +133,30 @@ export class StyleEvaluator {
 
     return stylesApplied;
   }
-  evaluateSwitchCondition(styleStates: Array<boolean>) {
+
+  evaluateSwitchCondition(stylesApplied: Set<number>) {
     let falsyBehavior = this.num();
+    let blockIndex = this.num();
+    let attribute = this.str();
     let currentValue = this.str(true, true);
-    let numValues = this.num();
-    let found = false;
-    let legal: Array<String> = [];
-    while (numValues--) {
-      let v = this.str();
-      legal.push(v);
-      let match = (v === currentValue);
-      found = found || match;
-      let numStyles = this.num();
-      while (numStyles--) {
-        let s = this.num();
-        if (match) styleStates[s] = true;
-      }
-    }
-    if (!found) {
-      if (!currentValue) {
-        if (falsyBehavior === FalsySwitchBehavior.error) {
-          throw new Error(`A value is required.`);
-        }
+    if (!currentValue) {
+      if (currentValue === null || falsyBehavior === FalsySwitchBehavior.unset) {
+        return;
       } else {
-        throw new Error(`"${currentValue} is not a known attribute value. Expected one of: ${legal.join(", ")}`);
+        throw new Error(`A value is required for ${attribute}.`);
       }
     }
+    let attrValue = this.blockGroups[blockIndex][attribute][currentValue];
+    if (attrValue === undefined) {
+      let legal = Object.keys(this.blockGroups[blockIndex][attribute]);
+      throw new Error(`"${currentValue} is not a known attribute value. Expected one of: ${legal.join(", ")}`);
+    }
+    let style = this.blockStyleIds[blockIndex][this.blockStyleIndices[blockIndex][attrValue]];
+    if (style) {
+      stylesApplied.add(style);
+    }
   }
+
   evaluateTernaryCondition(styleStates: Array<boolean>) {
     // Ternary supports multiple styles being enabled when true
     // and multiple values being enabled when false.
diff --git a/packages/@css-blocks/ember/src/TemplateAnalyzingRewriter.ts b/packages/@css-blocks/ember/src/TemplateAnalyzingRewriter.ts
@@ -1,4 +1,5 @@
 import {
+  Attribute,
   Block,
   Configuration as CSSBlocksConfiguration,
   Style,
@@ -248,68 +249,59 @@ class HelperInvocationGenerator {
     node.params = [];
     node.hash = hash();
     node.params.push(num(HelperInvocationGenerator.HELPER_VERSION));
-    let { blocks, blockParams } = this.buildBlockParams(sourceAnalysis);
+
+    let stylesUsed = new Array<Style>();
+    let blocksUsed = new Array<Block>();
+    let staticParams = this.buildStaticParams(sourceAnalysis, stylesUsed);
+    let ternaryParams = this.buildTernaryParams(sourceAnalysis, stylesUsed);
+    let booleanParams = this.buildBooleanParams(sourceAnalysis, stylesUsed);
+    let switchParams = this.buildSwitchParams(sourceAnalysis, blocksUsed);
+    let styleParams = this.buildStyleParams(sourceAnalysis, blocksUsed, stylesUsed);
+    let blockParams = this.buildBlockParams(blocksUsed);
+
     node.params.push(...blockParams);
-    let { styleIndices, styleParams } = this.buildStyleParams(sourceAnalysis, blocks);
     node.params.push(...styleParams);
-    node.params.push(...this.buildStaticParams(sourceAnalysis, styleIndices));
-    node.params.push(...this.buildTernaryParams(sourceAnalysis, styleIndices));
-    node.params.push(...this.buildBooleanParams(sourceAnalysis, styleIndices));
-    node.params.push(...this.buildSwitchParams(sourceAnalysis, styleIndices));
+    node.params.push(...staticParams );
+    node.params.push(...ternaryParams);
+    node.params.push(...booleanParams);
+    node.params.push(...switchParams);
     return node;
   }
 
-  indexOfBlock(blocks: Array<Block>, style: Style) {
-    for (let i = 0; i < blocks.length; i++) {
-      if (style.block === blocks[i] || style.block.isAncestorOf(blocks[i])) {
-        return i;
-      }
-    }
-    throw new Error("[internal error] Block not found.");
-  }
-
-  buildBlockParams(sourceAnalysis: ElementSourceAnalysis): {blocks: Array<Block>; blockParams: Array<AST.Expression>} {
+  buildBlockParams(blocksUsed: Array<Block>): Array<AST.Expression> {
     const { number: num, string: str, null: nullNode } = this.builders;
-    let blocks = [...sourceAnalysis.blocksFound];
     let blockParams = new Array<AST.Expression>();
-    blockParams.push(num(blocks.length));
-    for (let block of blocks) {
+    blockParams.push(num(blocksUsed.length));
+    for (let block of blocksUsed) {
       blockParams.push(str(block.guid));
       blockParams.push(nullNode()); // this could be a block when we implement block passing
     }
-    return {blocks, blockParams};
+    return blockParams;
   }
 
-  buildStyleParams(sourceAnalysis: ElementSourceAnalysis, blocks: Array<Block>): {styleIndices: Map<Style, number>; styleParams: Array<AST.Expression>} {
+  buildStyleParams(sourceAnalysis: ElementSourceAnalysis, blocks: Array<Block>, stylesUsed: Array<Style>): Array<AST.Expression> {
     const { number: num, string: str } = this.builders;
-    let styles = [...sourceAnalysis.stylesFound];
     let styleParams = new Array<AST.Expression>();
-    styleParams.push(num(styles.length));
-    let styleIndices = new Map<Style, number>();
-    let i = 0;
-    for (let style of styles) {
-      styleIndices.set(style, i++);
-      styleParams.push(num(this.indexOfBlock(blocks, style)));
+    styleParams.push(num(stylesUsed.length));
+    for (let style of stylesUsed) {
+      styleParams.push(num(blockIndex(blocks, style)));
       styleParams.push(str(style.asSource()));
     }
     styleParams.push(num(sourceAnalysis.size()));
-    return {
-      styleIndices,
-      styleParams,
-    };
+    return styleParams;
   }
 
-  buildStaticParams(sourceAnalysis: ElementSourceAnalysis, styleIndices: Map<Style, number>): Array<AST.Expression> {
+  buildStaticParams(sourceAnalysis: ElementSourceAnalysis, stylesUsed: Array<Style>): Array<AST.Expression> {
     const { number: num } = this.builders;
     let params = new Array<AST.Expression>();
     for (let style of sourceAnalysis.staticStyles) {
       params.push(num(StyleCondition.STATIC));
-      params.push(num(styleIndices.get(style)!));
+      params.push(num(styleIndex(stylesUsed, style)));
     }
     return params;
   }
 
-  buildTernaryParams(sourceAnalysis: ElementSourceAnalysis, styleIndices: Map<Style, number>): Array<AST.Expression> {
+  buildTernaryParams(sourceAnalysis: ElementSourceAnalysis, stylesUsed: Array<Style>): Array<AST.Expression> {
     const { number: num } = this.builders;
     let params = new Array<AST.Expression>();
     for (let ternaryClass of sourceAnalysis.ternaryStyles) {
@@ -322,7 +314,7 @@ class HelperInvocationGenerator {
       if (isTrueCondition(ternaryClass)) {
         params.push(num(ternaryClass.whenTrue.length));
         for (let cls of ternaryClass.whenTrue) {
-          params.push(num(styleIndices.get(cls)!));
+          params.push(num(styleIndex(stylesUsed, cls)));
         }
       } else {
         // there are no classes applied if true
@@ -331,7 +323,7 @@ class HelperInvocationGenerator {
       if (isFalseCondition(ternaryClass)) {
         params.push(num(ternaryClass.whenFalse.length));
         for (let cls of ternaryClass.whenFalse) {
-          params.push(num(styleIndices.get(cls)!));
+          params.push(num(styleIndex(stylesUsed, cls)));
         }
       } else {
         // there are no classes applied if false
@@ -341,7 +333,7 @@ class HelperInvocationGenerator {
     return params;
   }
 
-  buildBooleanParams(sourceAnalysis: ElementSourceAnalysis, styleIndices: Map<Style, number>): Array<AST.Expression> {
+  buildBooleanParams(sourceAnalysis: ElementSourceAnalysis, stylesUsed: Array<Style>): Array<AST.Expression> {
     const { number: num } = this.builders;
     let params = new Array<AST.Expression>();
     for (let dynamicAttr of sourceAnalysis.booleanStyles) {
@@ -353,13 +345,13 @@ class HelperInvocationGenerator {
       }
       params.push(num(dynamicAttr.value.size));
       for (let attr of dynamicAttr.value) {
-        params.push(num(styleIndices.get(attr)!));
+        params.push(num(styleIndex(stylesUsed, attr)));
       }
     }
     return params;
   }
 
-  buildSwitchParams(sourceAnalysis: ElementSourceAnalysis, styleIndices: Map<Style, number>): Array<AST.Expression> {
+  buildSwitchParams(sourceAnalysis: ElementSourceAnalysis, blocksUsed: Array<Block>): Array<AST.Expression> {
     const { number: num, string: str } = this.builders;
     let params = new Array<AST.Expression>();
 
@@ -371,24 +363,21 @@ class HelperInvocationGenerator {
       } else {
         params.push(num(FalsySwitchBehavior.unset));
       }
-      params.push(this.mustacheToStringExpression(this.builders, switchStyle.stringExpression!));
-      params.push(num(values.length));
+      // We have to find the attribute that belongs to the most specific sub-block.
+      let attr: Attribute | undefined;
       for (let value of values) {
-        let obj = switchStyle.group[value];
-        params.push(str(value));
-        // If there are values provided for this conditional, they are meant to be
-        // applied instead of the selected attribute group member.
-        if (switchStyle.value.size) {
-          params.push(num(switchStyle.value.size));
-          for (let val of switchStyle.value) {
-            params.push(num(styleIndices.get(val)!));
+        let attrValue = switchStyle.group[value];
+        if (!attr) {
+          attr = attrValue.attribute;
+        } else {
+          if (attr.block.isAncestorOf(attrValue.block)) {
+            attr = attrValue.attribute;
           }
         }
-        else {
-          params.push(num(1));
-          params.push(num(styleIndices.get(obj)!));
-        }
       }
+      params.push(num(blockIndex(blocksUsed, attr!)));
+      params.push(str(attr!.asSource()));
+      params.push(this.mustacheToStringExpression(this.builders, switchStyle.stringExpression!));
     }
     return params;
   }
@@ -432,3 +421,23 @@ class HelperInvocationGenerator {
     }
   }
 }
+
+function styleIndex(stylesUsed: Array<Style>, style: Style): number {
+  let index = stylesUsed.indexOf(style);
+  if (index >= 0) { return index; }
+  stylesUsed.push(style);
+  return stylesUsed.length - 1;
+}
+
+function blockIndex(blocks: Array<Block>, style: Style | Attribute) {
+  for (let i = 0; i < blocks.length; i++) {
+    if (style.block === blocks[i] || style.block.isAncestorOf(blocks[i])) {
+      return i;
+    } else if (blocks[i].isAncestorOf(style.block)) {
+      blocks[i] = style.block;
+      return i;
+    }
+  }
+  blocks.push(style.block);
+  return blocks.length - 1;
+}
diff --git a/packages/@css-blocks/ember/test/template-rewrite-test.ts b/packages/@css-blocks/ember/test/template-rewrite-test.ts
@@ -188,7 +188,7 @@ describe("Template Rewriting", function() {
       minify(`
         <div class={{-css-blocks 0 1 "${defaultBlock.guid}" null 1 0 ":scope" 1 1 0}}>
           <h1 class={{-css-blocks 0 1 "${headerBlock.guid}" null 1 0 ":scope" 1 1 0}}>Hello,
-          <span class={{-css-blocks 0 3 "${defaultBlock.guid}" null "${headerBlock.guid}" null "${typographyBlock.guid}" null 6 0 ".world" 1 ".emphasis" 2 ".underline" 0 ".world[thick]" 1 ".emphasis[style=bold]" 1 ".emphasis[style=italic]" 5 1 1 1 2 3 isWorld 1 0 0 2 (eq isThick 1) 1 3 4 1 textStyle 2 "bold" 1 4 "italic" 1 5}}>World</span>!</h1>
+          <span class={{-css-blocks 0 3 "${headerBlock.guid}" null "${typographyBlock.guid}" null "${defaultBlock.guid}" null 4 0 ".emphasis" 1 ".underline" 2 ".world" 2 ".world[thick]" 5 1 0 1 1 3 isWorld 1 2 0 2 (eq isThick 1) 1 3 4 1 0 ".emphasis[style]" textStyle}}>World</span>!</h1>
           <div class={{-css-blocks 0 1 "${defaultBlock.guid}" null 2 0 ".world" 0 ".planet" 1 3 isWorld 1 0 1 1}}>World</div>
           <div class={{-css-blocks 0 1 "${defaultBlock.guid}" null 2 0 ".planet" 0 ".world" 1 3 isWorld 1 0 1 1}}>World</div>
           <div class={{-css-blocks 0 1 "${defaultBlock.guid}" null 1 0 ".world" 1 3 isWorld 0 1 0}}>World</div>
@@ -334,7 +334,7 @@ describe("Template Rewriting", function() {
       minify(`
         <div class={{-css-blocks 0 1 "${defaultBlock.guid}" null 1 0 ":scope" 1 1 0}}>
           <h1 class={{-css-blocks 0 1 "${headerBlock.guid}" null 1 0 ":scope" 1 1 0}}>Hello,
-          <World cssClass={{-css-blocks 0 2 "${defaultBlock.guid}" null "${headerBlock.guid}" null 5 0 ".world" 1 ".emphasis" 0 ".world[thick]" 1 ".emphasis[style=bold]" 1 ".emphasis[style=italic]" 4 1 0 1 1 1 2 4 1 (textStyle) 2 "bold" 1 3 "italic" 1 4}} />!</h1>
+          <World cssClass={{-css-blocks 0 2 "${headerBlock.guid}" null "${defaultBlock.guid}" null 3 1 ".world" 0 ".emphasis" 1 ".world[thick]" 4 1 0 1 1 1 2 4 1 0 ".emphasis[style]" (textStyle)}} />!</h1>
         </div>
       `));
     let analysis = result.analysis.serialize();
@@ -400,7 +400,7 @@ describe("Template Rewriting", function() {
         <h1 class={{-css-blocks 0 1 "${headerBlock.guid}" null 1 0 ":scope" 1 1 0}}>Hello,
           {{yield (hash
             classnames=(hash
-              action=(-css-blocks 0 2 "${defaultBlock.guid}" null "${headerBlock.guid}" null 5 0 ".world" 1 ".emphasis" 0 ".world[thick]" 1 ".emphasis[style=bold]" 1 ".emphasis[style=italic]" 4 1 0 1 1 2 isThick 1 2 4 1 (textStyle) 2 "bold" 1 3 "italic" 1 4)))}}
+              action=(-css-blocks 0 2 "${headerBlock.guid}" null "${defaultBlock.guid}" null 3 1 ".world" 0 ".emphasis" 1 ".world[thick]" 4 1 0 1 1 2 isThick 1 2 4 1 0 ".emphasis[style]" (textStyle))))}}
         </h1>
       </div>
       `));
