refactor: GUID construction moves to BlockParser.

- The BlockParser is now responsible for constructing a GUID for each
Block, if needed. The Block constructor requires a GUID to be passed in.
- The BlockFactory is now responsible for ensuring each Block GUID is
unique. Once constructed, the BlockParser calls the BlockFactory to
register the new GUID. If it's not unique, registering throws an error.
- New config option allows for changing the number of significant
characters from the MD5 hash used when autogenerating GUIDs.
- Update tests to pass in GUID where we construct Blocks.
- Also, added some docs for public Block properties.

Author: Timothy Lindvall

Diff for: packages/@css-blocks/broccoli/test/Transport.ts

@@ -18,7 +18,7 @@ describe("Broccoli Transport Test", () => {
       types: {},
       collections: {},
     });
-    transport.blocks.add(new Block("foo", "bar"));
+    transport.blocks.add(new Block("foo", "bar", "test"));
     transport.mapping = {} as StyleMapping<"GlimmerTemplates.ResolvedFile">;
 
     transport.reset();

Diff for: packages/@css-blocks/core/src/Analyzer/Analysis.ts

@@ -360,7 +360,7 @@ export class Analysis<K extends keyof TemplateTypes> {
     // Create a temporary block so we can take advantage of `Block.lookup`
     // to easily resolve all BlockPaths referenced in the serialized analysis.
     // TODO: We may want to abstract this so we're not making a temporary block.
-    let localScope = new Block("analysis-block", "tmp");
+    let localScope = new Block("analysis-block", "tmp", "analysis-block");
     values.forEach(o => {
       analysis.blocks[o.name] = o.block;
       localScope.addBlockReference(o.name, o.block);

Diff for: packages/@css-blocks/core/src/BlockParser/BlockFactory.ts

@@ -47,6 +47,8 @@ export class BlockFactory {
   private paths: ObjectDictionary<string>;
   private preprocessQueue: PromiseQueue<PreprocessJob, ProcessedFile>;
 
+  private guids = new Set<string>();
+
   constructor(options: Options, postcssImpl = postcss, faultTolerant = false) {
     this.postcssImpl = postcssImpl;
     this.configuration = resolveConfiguration(options);
@@ -72,6 +74,7 @@ export class BlockFactory {
     this.paths = {};
     this.promises = {};
     this.blockNames = {};
+    this.guids.clear();
   }
 
   /**
@@ -413,6 +416,21 @@ export class BlockFactory {
     }
     return preprocessor;
   }
+
+  /**
+   * Registers a new GUID with the BlockFactory. If the given GUID has already been used, this
+   * method will throw an error.
+   * @param guid - The guid to register.
+   * @param identifier  - A reference to the file this block was generated from, for error reporting.
+   */
+  registerGuid(guid: string, identifier: FileIdentifier) {
+    if (this.guids.has(guid)) {
+      throw new CssBlockError("Block uses a GUID that has already been used! Check dependencies for conflicting GUIDs and/or increase the number of significant characters used to generate GUIDs.", {
+        filename: identifier,
+      });
+    }
+    this.guids.add(guid);
+  }
 }
 
 function sourceMapFromProcessedFile(result: ProcessedFile): RawSourceMap | string | undefined {

Diff for: packages/@css-blocks/core/src/BlockParser/BlockParser.ts

@@ -21,6 +21,7 @@ import { importBlocks } from "./features/import-blocks";
 import { processDebugStatements } from "./features/process-debug-statements";
 import { BlockFactory } from "./index";
 import { Syntax } from "./preprocessing";
+import { gen_guid } from "./utils/genGuid";
 
 const debug = debugGenerator("css-blocks:BlockParser");
 
@@ -62,11 +63,17 @@ export class BlockParser {
   /**
    * Main public interface of `BlockParser`. Given a PostCSS AST, returns a promise
    * for the new `Block` object.
-   * @param root  PostCSS AST
-   * @param sourceFile  Source file name
-   * @param defaultName Name of block
+   * @param root - PostCSS AST
+   * @param sourceFile - Source file name
+   * @param name - Name of block
+   * @param isDfnFile - Whether the block being parsed is a definition file. Definition files are incomplete blocks
+   *                    that will need to merge in rules from its Compiled CSS later. They are also expected to declare
+   *                    additional properties that regular Blocks don't, such as `block-id` and `block-interface-index`.
+   * @param expectedGuid - If a GUID is defined in the file, it's expected to match this value. This argument is only
+   *                       relevant to definition files, where the definition file is linked to Compiled CSS and
+   *                       both files may declare a GUID.
    */
-  public async parse(root: postcss.Root, identifier: string, name?: string, isDfnFile = false, expectedId?: string): Promise<Block> {
+  public async parse(root: postcss.Root, identifier: string, name?: string, isDfnFile = false, expectedGuid?: string): Promise<Block> {
     let importer = this.config.importer;
     let debugIdent = importer.debugIdentifier(identifier, this.config);
     let sourceFile = importer.filesystemPath(identifier, this.config) || debugIdent;
@@ -77,10 +84,11 @@ export class BlockParser {
     name = await discoverName(configuration, root, sourceFile, isDfnFile, name);
 
     // Discover the block's GUID, if it's a definition file.
-    const guid = await discoverGuid(configuration, root, sourceFile, isDfnFile, expectedId);
+    const guid = await discoverGuid(configuration, root, sourceFile, isDfnFile, expectedGuid) || gen_guid(identifier, configuration.guidAutogenCharacters);
+    this.factory.registerGuid(guid, identifier);
 
     // Create our new Block object and save reference to the raw AST.
-    let block = new Block(name, identifier, root, guid);
+    let block = new Block(name, identifier, guid, root);
 
     if (isDfnFile) {
       // Rules only checked when parsing definition files...

Diff for: packages/@css-blocks/core/src/BlockParser/utils/genGuid.ts

@@ -0,0 +1,26 @@
+import * as crypto from "crypto";
+import * as debugGenerator from "debug";
+import * as process from "process";
+
+const DEBUG = debugGenerator("css-blocks:caching");
+
+/**
+ * Generates a unique identifier from a given input identifier. This
+ * generated identifier hash will remain consistent for the tuple of (machine,
+ * user, css blocks installation location).
+ *
+ * @param  identifier Input Block identifier.
+ * @param  significantChars Number of characters from the start of the hash to use.
+ * @returns A GUID hash for the given Block identifier, sliced to the number of
+ *          significant characters given.
+ */
+export function gen_guid(identifier: string, signifcantChars: number): string {
+  let hash = crypto.createHash("md5")
+    .update(process.getuid().toString())
+    .update(__filename)
+    .update(identifier)
+    .digest("hex")
+    .slice(0, signifcantChars);
+  DEBUG("guid is %s for %s", hash, identifier);
+  return hash;
+}

Diff for: packages/@css-blocks/core/src/BlockTree/Block.ts

@@ -1,14 +1,11 @@
 import { MultiMap, ObjectDictionary } from "@opticss/util";
-import * as crypto from "crypto";
-import * as debugGenerator from "debug";
 import {
   CompoundSelector,
   ParsedSelector,
   parseSelector,
   postcss,
   postcssSelectorParser as selectorParser,
 } from "opticss";
-import * as process from "process";
 
 import { isAttributeNode, isClassNode } from "../BlockParser";
 import { isRootNode, toAttrToken } from "../BlockParser";
@@ -22,31 +19,25 @@ import { BlockClass } from "./BlockClass";
 import { Inheritable } from "./Inheritable";
 import { Styles } from "./Styles";
 
-const DEBUG = debugGenerator("css-blocks:caching");
-
 /**
- * Generates a 5 digit, unique identifier from a given input identifier. This
- * generated identifier hash will remain consistent for the tuple of (machine,
- * user, css blocks installation location).
- * @param  identifier Input Block identifier.
- * @returns This Block's guid hash.
+ * In-memory representation of a Block. If you're thinking of CSS Blocks
+ * in relation to the BEM architecture for CSS, this is the... well... "Block".
+ * Well, with a slight caveat....
+ *
+ * The Block is always the root node of a BlockTree. The Block may be the
+ * parent to any number of BlockClass nodes. Notably, the Block class only
+ * stores meta information about the block. Any CSS properties assigned to the
+ * `:scope` selector are stored on a special BlockClass node that is a child of
+ * the Block. You can access this node directly through the
+ * `rootClass` property.
+ *
+ * Block nodes store all data related to any `@block` imports, the
+ * `block-name`, implemented Blocks, the inherited Block, and any other
+ * metadata stored in the Block file.
  */
-function gen_guid(identifier: string): string {
-  let hash = crypto.createHash("md5")
-    .update(process.getuid().toString())
-    .update(__filename)
-    .update(identifier)
-    .digest("hex")
-    .slice(0, 5);
-  DEBUG("guid is %s for %s", hash, identifier);
-  return hash;
-}
-
 export class Block
   extends Inheritable<Block, Block, never, BlockClass> {
 
-  public static GUIDS_USED = new Map<string, string[]>();
-
   private _blockReferences: ObjectDictionary<Block> = {};
   private _blockReferencesReverseLookup: Map<Block, string> = new Map();
   private _blockExports: ObjectDictionary<Block> = {};
@@ -56,6 +47,14 @@ export class Block
   private _blockErrors: CssBlockError[] = [];
   private hasHadNameReset = false;
 
+  /**
+   * A unique identifier for this Block. Generally created from a hash
+   * of the FileIdentifier and other process information.
+   *
+   * For caching to work properly, this GUID must be unique to the block and
+   * shouldn't change between recompiles. You shouldn't use file contents to
+   * create this hash.
+   */
   public readonly guid: string;
 
   /**
@@ -65,52 +64,36 @@ export class Block
    */
   private _dependencies: Set<string>;
 
+  /**
+   * A direct reference to the BlockClass that holds style information for the
+   * `:scope` selector of this Block. The rootClass is also available through
+   * other traversal methods, as you would access any other BlockClass that
+   * belongs to this Block.
+   */
   public readonly rootClass: BlockClass;
+
+  /**
+   * The PostCSS AST of the stylesheet this Block was built from. Used
+   * primarily for error reporting, if present.
+   */
   public stylesheet: postcss.Root | undefined;
 
-  constructor(name: string, identifier: FileIdentifier, stylesheet?: postcss.Root, incomingGuid?: string) {
+  /**
+   * Creates a new Block.
+   *
+   * @param name - The default name for this block. This can be reset once (and only once)
+   *               using the `setName()` method.
+   * @param identifier - An unique ID referencing the file/blob this Block is created from.
+   * @param guid - The GUID for this block. This GUID should be unique. (BlockFactory is
+   *               responsible for enforcing uniqueness.)
+   * @param stylesheet - The PostCSS AST of the stylesheet this block is built from.
+   */
+  constructor(name: string, identifier: FileIdentifier, guid: string, stylesheet?: postcss.Root) {
     super(name);
     this._identifier = identifier;
     this._dependencies = new Set<string>();
     this.rootClass = new BlockClass(ROOT_CLASS, this);
     this.stylesheet = stylesheet;
-
-    // If we found a GUID from the :scope rule, use that. Otherwise, generate one.
-    let guid = incomingGuid || gen_guid(identifier);
-
-    // We insist that each block have a unique GUID. In the event that we've somehow
-    // ended up with two blocks that have the same GUID, our options depend on whether
-    // we got a preset GUID passed in or not.
-    if (Block.GUIDS_USED.has(guid)) {
-      if (incomingGuid) {
-        // If the GUID was already generated prior to creating the block, we have to
-        // bail out. (There's likely Compiled CSS that depends on that GUID.)
-        throw new CssBlockError("Block uses a GUID that has already been used!", {
-          filename: identifier,
-        });
-      } else {
-        // Ok, we autogenerated this. Let's append some gobbledygook to recover.
-        const guidBaseList = Block.GUIDS_USED.get(guid);
-        if (!guidBaseList) {
-          // Ah crumbs. There should be a list but there isn't.
-          throw new CssBlockError("Block uses a GUID that has already been used!", {
-            filename: identifier,
-          });
-        }
-        guid = `${guid}${guidBaseList.length}`;
-        if (guidBaseList.includes(guid) || Block.GUIDS_USED.has(guid)) {
-          // Ah crumbs. Our safety check to make sure the GUID really hasn't been used failed.
-          throw new CssBlockError("Block uses a GUID that has already been used!", {
-            filename: identifier,
-          });
-        }
-        // Phew, okay, appending a numerical id works. Let's go with that.
-        guidBaseList.push(guid);
-      }
-    } else {
-      // Hey, this is unique! Cool, let's remember it in case we see it again later.
-      Block.GUIDS_USED.set(guid, []);
-    }
     this.guid = guid;
 
     this.addClass(this.rootClass);

Diff for: packages/@css-blocks/core/src/configuration/resolver.ts

@@ -33,6 +33,7 @@ const DEFAULTS: ResolvedConfiguration = {
   preprocessors: {},
   disablePreprocessChaining: false,
   maxConcurrentCompiles: 4,
+  guidAutogenCharacters: 5,
 };
 
 /**
@@ -86,6 +87,9 @@ class Resolver implements ResolvedConfiguration {
   get maxConcurrentCompiles(): number {
     return this._opts.maxConcurrentCompiles;
   }
+  get guidAutogenCharacters(): number {
+    return this._opts.guidAutogenCharacters;
+  }
 }
 
 export function resolveConfiguration(

Diff for: packages/@css-blocks/core/src/configuration/types.ts

@@ -31,6 +31,21 @@ export interface Configuration {
    * this can be disabled by setting `disablePreprocessChaining` to true.
    */
   disablePreprocessChaining: boolean;
+
+  /**
+   * Determines the number of significant characters used when generating GUIDs for blocks.
+   * These GUIDs are build using a hash based on the file's unique identifier (usually an
+   * absolute file path).
+   *
+   * The default value used is 5 characters. Normally you shouldn't need to change this,
+   * but you can increase the number of significant characters in the exceedingly rare event
+   * you run into GUID conflicts.
+   *
+   * This value does not affect GUIDs from pre-compiled blocks imported from other dependencies.
+   * In these cases, the Block ID is pre-determined using the GUID value in the block's
+   * definition file.
+   */
+  guidAutogenCharacters: number;
 }
 
 /**
@@ -50,4 +65,5 @@ export type ConfigurationSimpleKeys = "outputMode"
                                     | "importer"
                                     | "rootDir"
                                     | "disablePreprocessChaining"
-                                    | "maxConcurrentCompiles";
+                                    | "maxConcurrentCompiles"
+                                    | "guidAutogenCharacters";

Diff for: packages/@css-blocks/core/test/Block/inheritable-test.ts

@@ -60,7 +60,7 @@ class TestSource extends Inheritable<
 
 type ContainerNode = Inheritable<TestNode, TestSource, TestSource, TestSink>;
 
-const TEST_BLOCK = new Block("test", "tree");
+const TEST_BLOCK = new Block("test", "tree", "test");
 class TestNode extends Inheritable<
   TestNode,   // Self
   TestSource, // Root