chore: Code cleanup.

 - Added comments and jsdoc annotations.
 - Template Analyzers use `startElement` and `endElement` APIs.
 - Rewriters' element analyzers now use special syntax, easily refactored later when we address #84.
 - Old `addElement` method now gone from base Analysys object.
 - Annotate more broccoli-css-blocks future features in comments.

Author: amiller-gh

packages/css-blocks/src/Analyzer/Analysis.ts

@@ -12,10 +12,7 @@ import {
   TemplateIntegrationOptions,
   TemplateTypes,
 } from "@opticss/template-api";
-import {
-  ObjectDictionary,
-  objectValues,
-} from "@opticss/util";
+import { ObjectDictionary, objectValues } from "@opticss/util";
 import { IdentGenerator } from "opticss";
 
 import { BlockFactory } from "../BlockParser";
@@ -28,7 +25,7 @@ import { TemplateValidator, TemplateValidatorOptions } from "./validations";
 
 /**
  * This interface defines a JSON friendly serialization
- * of a {TemplateAnalysis}.
+ * of an {Analysis}.
  */
 export interface SerializedAnalysis {
   template: SerializedTemplateInfo<keyof TemplateTypes>;
@@ -39,8 +36,8 @@ export interface SerializedAnalysis {
 }
 
 /**
- * A TemplateAnalysis performs book keeping and ensures internal consistency of the block objects referenced
- * within a template. It is designed to be used as part of an AST walk over a template.
+ * An Analysis performs book keeping and ensures internal consistency of the block objects referenced
+ * within a single template. It is designed to be used as part of an AST walk over a template.
  *
  * 1. Call [[startElement startElement()]] at the beginning of an new html element.
  * 2. Call [[addStyle addStyle(style, isDynamic)]] for all the styles used on the current html element.
@@ -93,22 +90,39 @@ export class Analysis<K extends keyof TemplateTypes> {
 
   /**
    * Return the number of blocks discovered in this Template.
-   */
+  */
   blockCount(): number { return Object.keys(this.blocks).length; }
 
   /**
    * Convenience setter for adding a block to the template scope.
    */
-  addBlock(name: string, block: Block) { this.blocks[name] = block; }
+  addBlock(name: string, block: Block): void { this.blocks[name] = block; }
 
   /**
    * Convenience getter for fetching a block from the template scope.
    */
-  getBlock(name: string): Block { return this.blocks[name]; }
+  getBlock(name: string): Block | undefined { return this.blocks[name]; }
+
+  /**
+   * Return the number of elements discovered in this Analysis.
+   */
+  elementCount(): number { return this.elements.size; }
+
+  /**
+   * Get the nth element discovered in this Analysis.
+   */
+  getElement<BooleanExpression, StringExpression, TernaryExpression>(idx: number): ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression> {
+    let mapIter = this.elements.entries();
+    let el = mapIter.next().value;
+    for (let i = 0; i < idx; i++) {
+      el = mapIter.next().value;
+    }
+    return el[1];
+  }
 
   /**
    * Return the number of styles discovered in this Analysis' Template.
-   * This is slow.
+   * TODO: This is slow. We can pre-compute this as elements are added.
    */
   styleCount(): number {
     let c = 0;
@@ -124,25 +138,11 @@ export class Analysis<K extends keyof TemplateTypes> {
   }
 
   /**
-   * Return the number of elements discovered in this Analysis.
-   */
-  elementCount(): number { return this.elements.size; }
-
-  /**
-   * Get the nth element discovered in this Analysis.
+   * Get either the owner Analyzer's optimization options, or a default set of options.
    */
-  getElement<BooleanExpression, StringExpression, TernaryExpression>(idx: number): ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression> {
-    let mapIter = this.elements.entries();
-    let el = mapIter.next().value;
-    for (let i = 0; i < idx; i++) {
-      el = mapIter.next().value;
-    }
-    return el[1];
-  }
-
   optimizationOptions(): TemplateIntegrationOptions {
-    // TODO: take this as an argument from the template integration.
-    return {
+    // TODO: Optimization options must be handled / propagated better.
+    return this.parent ? this.parent.optimizationOptions : {
       rewriteIdents: {
         id: false,
         class: true,
@@ -164,6 +164,7 @@ export class Analysis<K extends keyof TemplateTypes> {
   }
 
   /**
+   * Returns the local name of the provided in this Analysis' template.
    * @param block The block for which the local name should be returned.
    * @return The local name of the given block.
    */
@@ -184,46 +185,42 @@ export class Analysis<K extends keyof TemplateTypes> {
     return null;
   }
 
+  /**
+   * Get a new {ElementAnalysis} object to track an individual element's {Style}
+   * consumption in this {Analysis}' template. Every {ElementAnalysis} returned from
+   * `Analysis.startElement` must be passed to `Analysis.endElement` before startElement
+   * is called again.
+   * @param location  The {SourceLocation} of this element in the template.
+   * @param tagName  Optional. The tag name of the element being represented by this {ElementAnalysis}.
+   * @returns A new {ElementAnalysis}.
+   */
   startElement<BooleanExpression, StringExpression, TernaryExpression>(location: SourceLocation | SourcePosition, tagName?: string): ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression> {
-    if (isSourcePosition(location)) {
-      location = {start: location};
-    }
+    if (isSourcePosition(location)) { location = {start: location}; }
     if (this.currentElement) {
       throw new Error("Internal Error: failure to call endElement before previous call to startElement.");
     }
     this.currentElement = new ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression>(location, tagName, this.idGenerator.nextIdent());
     return this.currentElement;
   }
 
-  endElement<BooleanExpression, StringExpression, TernaryExpression>(element: ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression>, endPosition?: SourcePosition) {
+  /**
+   * Finish an {ElementAnalysis} object returned from `Analysis.startElement` to
+   * the end location in source and save {Style} usage on the parent {Analysis}.
+   * @param element  The {ElementAnalysis} we are finishing.
+   * @param endPosition  Optional. The location in code where this element tag is closed..
+   */
+  endElement<BooleanExpression, StringExpression, TernaryExpression>(element: ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression>, endPosition?: SourcePosition): void {
     if (this.currentElement !== element) {
       throw new Error("Element is not the current element.");
     }
-    if (endPosition) {
-      element.sourceLocation.end = endPosition;
-    }
-    this.addElement(element);
-    this.currentElement = undefined;
-  }
-
-  private addFilename(pos: SourcePosition | undefined) {
-    if (pos && !pos.filename) {
-      pos.filename = this.template.identifier;
-    }
-  }
-
-  addElement<BooleanExpression, StringExpression, TernaryExpression>(element: ElementAnalysis<BooleanExpression, StringExpression, TernaryExpression>) {
-    if (!element.id) {
-      element.id = this.idGenerator.nextIdent();
-    }
+    if (endPosition) { element.sourceLocation.end = endPosition; }
+    if (!element.id) { element.id = this.idGenerator.nextIdent(); }
     if (this.elements.get(element.id)) {
       throw new Error(`Internal Error: an element with id = ${element.id} already exists in this analysis`);
     }
-    this.addFilename(element.sourceLocation.start);
-    this.addFilename(element.sourceLocation.end);
-    if (!element.sealed) {
-      element.seal();
-    }
+    this.ensureFilename(element.sourceLocation.start);
+    this.ensureFilename(element.sourceLocation.end);
+    if (!element.sealed) { element.seal(); }
     this.validator.validate(this, element);
     this.elements.set(element.id, element);
     if (this.parent) {
@@ -234,6 +231,18 @@ export class Analysis<K extends keyof TemplateTypes> {
         this.parent.saveDynamicStyle(s, this);
       }
     }
+    this.currentElement = undefined;
+  }
+
+  /**
+   * Given a {SourcePosition}, ensure that the file name is present. If not,
+   * add the template identifier.
+   * @param post  The {SourcePosition} we are validating.
+   */
+  private ensureFilename(pos: SourcePosition | undefined) {
+    if (pos && !pos.filename) {
+      pos.filename = this.template.identifier;
+    }
   }
 
   /**

packages/glimmer-templates/src/Analyzer.ts

@@ -102,13 +102,12 @@ export class GlimmerAnalyzer extends Analyzer<TEMPLATE_TYPE> {
     let localBlockNames = Object.keys(analysis.blocks).map(n => n === "" ? "<default>" : n);
     self.debug(`Analyzing ${componentName}. ${localBlockNames.length} blocks in scope: ${localBlockNames.join(", ")}.`);
 
-    let elementAnalyzer = new ElementAnalyzer(block, template, this.options);
+    let elementAnalyzer = new ElementAnalyzer(analysis, this.options);
     traverse(ast, {
       ElementNode(node) {
         elementCount++;
         let atRootElement = (elementCount === 1);
         let element = elementAnalyzer.analyze(node, atRootElement);
-        analysis.addElement(element);
         if (self.debug.enabled) self.debug("Element analyzed:", element.forOptimizer(self.options).toString());
       },
     });

packages/glimmer-templates/src/ElementAnalyzer.ts

@@ -4,6 +4,7 @@ import { assertNever } from "@opticss/util";
 import { AttrValue, Block, BlockClass, DynamicClasses, ElementAnalysis, ResolvedConfiguration as CSSBlocksConfiguration } from "css-blocks";
 import * as debugGenerator from "debug";
 
+import { GlimmerAnalysis } from "./Analyzer";
 import { ResolvedFile } from "./GlimmerProject";
 import { cssBlockError } from "./utils";
 
@@ -29,17 +30,21 @@ const STYLE_UNLESS: "style-unless" = "style-unless";
 const debug = debugGenerator("css-blocks:glimmer:analyzer");
 
 export class ElementAnalyzer {
+  analysis: GlimmerAnalysis;
+  block: Block;
   template: ResolvedFile;
   cssBlocksOpts: CSSBlocksConfiguration;
-  block: Block;
-  constructor(block: Block, template: ResolvedFile, cssBlocksOpts: CSSBlocksConfiguration) {
-    this.block = block;
-    this.template = template;
+
+  constructor(analysis: GlimmerAnalysis, cssBlocksOpts: CSSBlocksConfiguration) {
+    this.analysis = analysis;
+    this.block = analysis.blocks[""];
+    this.template = analysis.template;
     this.cssBlocksOpts = cssBlocksOpts;
   }
   analyze(node: AST.ElementNode, atRootElement: boolean): AnalysisElement {
-    let element = new ElementAnalysis<null, null, null>(nodeLocation(node), node.tag);
+    let element = this.analysis.startElement<null, null, null>(nodeLocation(node), node.tag);
     this._analyze(node, atRootElement, {element, storeConditionals: false});
+    this.analysis.endElement(element);
     return element;
   }
   analyzeForRewrite(node: AST.ElementNode, atRootElement: boolean): TemplateElement {

packages/glimmer-templates/src/Rewriter.ts

@@ -48,7 +48,7 @@ export class GlimmerRewriter implements ASTPlugin {
     this.styleMapping  = styleMapping;
     this.cssBlocksOpts = resolveConfiguration(cssBlocksOpts);
     this.elementCount  = 0;
-    this.elementAnalyzer = new ElementAnalyzer(this.block, this.template, this.cssBlocksOpts);
+    this.elementAnalyzer = new ElementAnalyzer(this.analysis, this.cssBlocksOpts);
   }
 
   debug(message: string, ...args: whatever[]): void {
@@ -65,6 +65,10 @@ export class GlimmerRewriter implements ASTPlugin {
   ElementNode(node: AST.ElementNode) {
     this.elementCount++;
     let atRootElement = (this.elementCount === 1);
+    // TODO: We use this to re-analyze elements in the rewriter.
+    //       We've already done this work and should be able to
+    //       re-use the data! Unfortunately, there are problems...
+    //       See: https://github.com/css-blocks/css-blocks/issues/84
     let element = this.elementAnalyzer.analyzeForRewrite(node, atRootElement);
     this.debug(element.forOptimizer(this.cssBlocksOpts)[0].toString());
     let rewrite = this.styleMapping.simpleRewriteMapping(element);