chore(css-blocks): Code quality improvements.

 - Disentangle Block from opticss
 - Remove SelectorFactory concept in favor of better caching in parseSelector (see opticss PR)
 - Comment all BlockTree modules
 - Add BlockTree tests
 - Start Style object / RulesetContainer tests
 - Begin crafting final Node.lookup interface for BlockTrees
 - Add optional non-silent mode to Node.lookup. If an error occurs during lookup, request an error to be thrown.

Author: amiller-gh

packages/css-blocks/src/Block/Block.ts

@@ -4,7 +4,6 @@ import {
   CompoundSelector,
   ParsedSelector,
   parseSelector,
-  SelectorFactory,
 } from "opticss";
 import * as postcss from "postcss";
 import selectorParser = require("postcss-selector-parser");
@@ -16,15 +15,15 @@ import {
   NodeAndType,
   stateParser,
 } from "../BlockParser";
-import { CLASS_NAME_IDENT, ROOT_CLASS } from "../BlockSyntax";
-import { BlockPath } from "../BlockSyntax";
+import { BlockPath, CLASS_NAME_IDENT, ROOT_CLASS } from "../BlockSyntax";
 import { OptionsReader } from "../OptionsReader";
-import { CssBlockError } from "../errors";
+import { SourceLocation } from "../SourceLocation";
+import { CssBlockError, InvalidBlockSyntax } from "../errors";
 import { FileIdentifier } from "../importing";
 import { LocalScopedContext } from "../util/LocalScope";
 
 import { BlockClass } from "./BlockClass";
-import { SourceNode } from "./BlockTree";
+import { SourceContainer } from "./BlockTree";
 import { State } from "./State";
 
 export type Style = BlockClass | State;
@@ -38,9 +37,7 @@ export const OBJ_REF_SPLITTER = (s: string): [string, string] | undefined => {
   return;
 };
 
-export class Block
-  extends SourceNode<Block, BlockClass>
-  implements SelectorFactory {
+export class Block extends SourceContainer<Block, BlockClass> {
   private _rootClass: BlockClass;
   private _blockReferences: ObjectDictionary<Block> = {};
   private _blockReferencesReverseLookup: Map<Block, string> = new Map();
@@ -51,7 +48,7 @@ export class Block
   /**
    * array of paths that this block depends on and, if changed, would
    * invalidate the compiled css of this block. This is usually only useful in
-   * preprocessed blocks.
+   * pre-processed blocks.
    */
   private _dependencies: Set<string>;
 
@@ -90,6 +87,7 @@ export class Block
       return;
     }
   }
+
   lookupLocal(name: string): Style | undefined {
     let blockRef = this._blockReferences[name];
     if (blockRef) {
@@ -120,26 +118,27 @@ export class Block
    *   A single dot by itself returns the current block.
    * @returns The Style referenced at the supplied path.
    */
-  public lookup(path: string | BlockPath): Style | undefined {
+  public lookup(path: string | BlockPath, errLoc?: SourceLocation): Style | undefined {
     path = new BlockPath(path);
     let block = this.getReferencedBlock(path.block);
-    if (!block) return undefined;
+    if (!block) {
+      if (errLoc) { throw new InvalidBlockSyntax(`No Block named "${path.block}" found in scope.`, errLoc); }
+      return undefined;
+    }
     let klass = block.resolveClass(path.class);
-    if (!klass) return undefined;
     let state;
-    if (path.state) {
+    if (klass && path.state) {
       let groupName = path.state.group ? path.state.group : path.state.name;
       let stateName = path.state.group ? path.state.name : undefined;
       state = klass.resolveState(groupName, stateName);
       if (!state) return undefined;
     }
-    return state || klass;
 
-    // for (let part of path.tokens()) {
+    if (!state && !klass && errLoc) {
+      throw new InvalidBlockSyntax(`No Style "${path.path}" found on Block "${block.name}".`, errLoc);
+    }
 
-    //   res = res.resolveChild(part);
-    //   if (!res) { break; }
-    // }
+    return state || klass || undefined;
   }
 
   /// End of methods to implement LocalScope<Block, Style>
@@ -473,23 +472,6 @@ export class Block
     return false;
   }
 
-  /**
-   * TODO: Move selector cache into `parseSelector` and have consumers of this
-   *       method interface with the parseSelector utility directly.
-   * Given a PostCSS Rule, ensure it is present in this Block's parsed rule
-   * selectors cache, and return the ParsedSelector array.
-   * @param rule  PostCSS Rule
-   * @return ParsedSelector array
-   */
-  getParsedSelectors(rule: postcss.Rule): ParsedSelector[] {
-    let sels = this.parsedRuleSelectors.get(rule);
-    if (!sels) {
-      sels = parseSelector(rule);
-      this.parsedRuleSelectors.set(rule, sels);
-    }
-    return sels;
-  }
-
   /**
    * Objects that contain Blocks are often passed into assorted libraries' options
    * hashes. Some libraries like to `JSON.stringify()` their options to create

packages/css-blocks/src/Block/BlockClass.ts

@@ -108,6 +108,9 @@ export class BlockClass extends StyleNode<BlockClass, Block, Block, StateGroup>
     }
   }
 
+  // TODO: Implement lookup relative to BlockClass.
+  public lookup(): undefined { return undefined; }
+
   /**
    * Return array self and all children.
    * @param shallow Pass false to not include children.

packages/css-blocks/src/Block/BlockTree/Inheritable.ts

@@ -1,16 +1,32 @@
+/**
+ * The `Inheritable` module delivers the core data structure for making type safe,
+ * single-typed-children, tree nodes.
+ *
+ * This abstract class, in order to support tree nodes of *any* kind, makes creative
+ * use of the `any` type in a few places – but don't be fooled! These typing holes
+ * are *required* to be plugged by any by any implementing class by virtue of the type
+ * generics they are forced to provide when extending. By the time this abstract class
+ * is exposed to the outside world, all typing holes have been plugged and we are left
+ * with a fully typed tree structure.
+ *
+ * @module Block/BlockTree/Inheritable
+ */
 import { ObjectDictionary } from "@opticss/util";
 import { whatever } from "@opticss/util";
 
+import { SourceLocation } from "../../SourceLocation";
+
 /* tslint:disable:prefer-whatever-to-any */
 export type AnyNode = Inheritable<any, any, any, any>;
 
 export abstract class Inheritable<
   Self extends Inheritable<Self, Root, Parent, Child>,
-  Root extends Inheritable<any, Root, null, any> | Self,
-  Parent extends Inheritable<any, Root, any, Self> | null,
-  Child extends Inheritable<any, Root, Self, any> | null
+  Root extends Inheritable<any, Root, null, AnyNode> | Self,
+  Parent extends Inheritable<any, Root, AnyNode | null, Self> | null,
+  Child extends Inheritable<any, Root, Self, AnyNode | null> | null
 > {
 /* tslint:enable:prefer-whatever-to-any */
+
   protected _name: string;
   protected _base: Self | undefined;
   protected _root: Root | Self;
@@ -23,17 +39,6 @@ export abstract class Inheritable<
    */
   protected abstract newChild(name: string): Child;
 
-  // // TODO: Currently only ever returns itself if is a style. Need to get it to look other things up.
-  // public lookup(path: string | BlockPath): Self | Child | Descendants | undefined {
-  //   path = new BlockPath(path);
-  //   let res: Self | Child | Descendants | null = this.asSelf();
-  //   for (let part of path.tokens()) {
-  //     res = res.resolveChild(part);
-  //     if (!res) { break; }
-  //   }
-  //   return res ? res : undefined;
-  // }
-
   /**
    * Inheritable constructor
    * @param name Name for this Inheritable instance.
@@ -74,14 +79,28 @@ export abstract class Inheritable<
   }
 
   /**
-   * traverse parents and return the base block object.
-   * @returns The base block in this container tree.
+   * Traverse parents and return the base block object.
+   * @returns The base node in this tree.
    */
   public get root(): Root {
     // This is a safe cast because we know root will only be set to `Self` for `Source` nodes.
     return this._root as Root;
   }
 
+  /**
+   * The `block` property is an alias for `root`. This isn't the dryest place to put
+   * this line, but every extension re-declared this interface itself and I wanted it
+   * in one place.
+   * @returns The base node in this tree.
+   */
+  public get block(): Root { return this.root; }
+
+  public abstract lookup(path: string, errLoc?: SourceLocation): AnyNode | undefined;
+
+  /**
+   * Sets the base node that this node inherits from. Must be of same type.
+   * @prop  base  Self  The new base node.
+   */
   public setBase(base: Self) {
     this._base = base;
   }
@@ -92,6 +111,7 @@ export abstract class Inheritable<
    * other relationships to this object.
    *
    * If nothing is inherited, this returns an empty array.
+   * @returns The array of nodes this node inherits from.
    */
   public resolveInheritance(): Self[] {
     let inherited: Self[] = [];
@@ -107,6 +127,7 @@ export abstract class Inheritable<
    * Resolves the child with the given name from this node's inheritance
    * chain. Returns undefined if the child is not found.
    * @param name The name of the child to resolve.
+   * @returns The child node, or `null`
    */
   public resolveChild(name: string): Child | null {
     let state: Child | null = this.getChild(name);
@@ -118,32 +139,32 @@ export abstract class Inheritable<
     return state || null;
   }
 
-  // /**
-  //  * Find the closest common ancestor Block between two Styles
-  //  * TODO: I think there is a more efficient way to do this.
-  //  * @param relative  Style to compare ancestry with.
-  //  * @returns The Style's common Block ancestor, or null.
-  //  */
-  // commonAncestor(relative: Style<Child>): Style<Child> | null {
-  //   let blockChain = new Set(...this.block.rootClass.resolveInheritance()); // lol
-  //   blockChain.add(this.block.rootClass);
-  //   let common = [relative.block.rootClass, ...relative.block.rootClass.resolveInheritance()].filter(b => blockChain.has(b));
-  //   return common.length ? common[0] as Style<Child> : null;
-  // }
-
   /**
-   * Given a parent that is a base class of this style, retrieve this style's
-   * base style from it, if it exists. This method does not traverse into base styles.
+   * Retrieve a child node from this object at `key`.
+   * @param key string  The key to fetch the child object from.
+   * @returns The child node.
    */
   public getChild(key: string): Child | null {
     return this._children.get(key) || null;
   }
 
+  /**
+   * Set a child node on this object at `key`.
+   * @param key string  The key to set the child object to.
+   * @returns The child node.
+   */
   public setChild(key: string, value: Child): Child {
     this._children.set(key, value);
     return value;
   }
 
+  /**
+   * Ensure a child node exists on this object at `key`. If it does not, create it.
+   * If `key` is not provided, use the child name as the key.
+   * @param name string  The name of this object to ensure.
+   * @param key string  The key at which this child object should be (optional)
+   * @returns The child node.
+   */
   public ensureChild(name: string, key?: string): Child {
     key = key !== undefined ? key : name;
     if (!this._children.has(name)) {
@@ -152,15 +173,27 @@ export abstract class Inheritable<
     return this._children.get(key) as Child;
   }
 
+  /**
+   * Returns an array of all children nodes in the order they were added.
+   * @returns The children array.
+   */
   public children(): Child[] {
     return [...this._children.values()];
   }
 
+  /**
+   * Returns a map of all children nodes at the keys they are stored..
+   * @returns The children map.
+   */
   public childrenMap(): Map<string, Child> {
     return new Map(this._children);
   }
 
-  // TODO: Cache this maybe? Convert entire model to only use hash?...
+  /**
+   * Returns a hash of all children nodes at the keys they are stored..
+   * TODO: Cache this maybe? Convert entire model to only use hash?...
+   * @returns The children hash.
+   */
   public childrenHash(): ObjectDictionary<Child> {
     let out = {};
     for (let [key, value] of this._children.entries()) {