fix: Updates per CR feedback.

- Better logic for merging CSS decls onto CSS Block nodes.
- discoverGuid doesn't need to be async.
- Don't check if each rule in the blockdef file has a block-class decl.
Instead, validate that each style node has a preset class name after the
block has been processed.
- Add new prop names to BlockSyntax.
- Rename some methods and properties.
- Allow spaces preceeding @block-syntax-version.
- Add fixtures and test coverage for blockdef processing. Ensure that
the resulting block reflects the decls on a given style node for property
conflict resolution.

Author: Timothy Lindvall

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

@@ -1,6 +1,6 @@
 import { ObjectDictionary, isString } from "@opticss/util";
 import * as debugGenerator from "debug";
-import { LegacyRawSourceMap, adaptFromLegacySourceMap, postcss } from "opticss";
+import { LegacyRawSourceMap, adaptFromLegacySourceMap, parseSelector, postcss } from "opticss";
 import * as path from "path";
 import { RawSourceMap } from "source-map";
 
@@ -9,7 +9,6 @@ import { Options, ResolvedConfiguration, resolveConfiguration } from "../configu
 import { CssBlockError } from "../errors";
 import { FileIdentifier, ImportedCompiledCssFile, ImportedFile, Importer } from "../importing";
 import { upgradeDefinitionFileSyntax } from "../PrecompiledDefinitions/block-syntax-version";
-import { sourceRange } from "../SourceLocation";
 import { PromiseQueue } from "../util/PromiseQueue";
 
 import { BlockParser, ParsedSource } from "./BlockParser";
@@ -365,29 +364,39 @@ export class BlockFactory {
     const block = await this.parser.parseDefinitionSource(definitionAst, file.definitionIdentifier, file.blockId, file.defaultName);
 
     // Merge the rules from the CSS contents into the Block.
-    const styleNodesMap = block.compiledClassesMap(true);
+    const styleNodesMap = block.presetClassesMap(true);
     cssContentsAst.walkRules(rule => {
-      rule.selectors.forEach(sel => {
-        if (sel.split(".").length !== 1 || !sel.startsWith(".")) {
-          // Skip it, we only care about selectors with only one class.
-          return;
-        }
-        const styleNode = styleNodesMap[sel];
-        if (!styleNode) {
-          block.addError(
-            new CssBlockError(
-              `Selector ${sel} exists in Compiled CSS file but doesn't match any rules in definition file.`,
-              sourceRange(this.configuration, cssContentsAst.root(), file.identifier, rule),
-            ),
-          );
-          return;
+      const parsedSelectors = parseSelector(rule);
+
+      parsedSelectors.forEach(sel => {
+        let doProcess = true;
+
+        // Check selector: do not process selectors with class names that
+        // aren't from this block. (aka: resolution selectors)
+        sel.eachSelectorNode(selNode => {
+          if (selNode.type !== "class") {
+            return;
+          }
+          if (!styleNodesMap[selNode.value]) {
+            doProcess = false;
+            return;
+          }
+        });
+        // If this node should be processed, add its declarations to each class
+        // in the key selector (the last part of the selector).
+        // We add the declarations so that later we can determine any conflicts
+        // between the imported CSS and any app CSS that relies on it.
+        if (doProcess) {
+          const keys = sel.key.nodes;
+          keys.forEach(key => {
+            if (key.value && key.type === "class") {
+              styleNodesMap[key.value].rulesets.addRuleset(this.configuration, file.identifier, rule);
+            }
+          });
         }
-        styleNode.rulesets.addRuleset(this.configuration, file.identifier, rule);
       });
     });
 
-    // TODO: Set the block's name from the block-name rule. (We skip this later for definition files.)
-
     // And we're done!
     return block;
   }

packages/@css-blocks/core/src/BlockParser/BlockParser.ts

@@ -93,7 +93,7 @@ export class BlockParser {
     let guid: string;
     let guidDiscoveryError: CssBlockError | undefined;
     try {
-      guid = await discoverGuid(configuration, root, sourceFile, isDfnFile, expectedGuid) || gen_guid(identifier, configuration.guidAutogenCharacters);
+      guid = discoverGuid(configuration, root, sourceFile, isDfnFile, expectedGuid) || gen_guid(identifier, configuration.guidAutogenCharacters);
     } catch (e) {
       guidDiscoveryError = e;
       guid = gen_guid(identifier, configuration.guidAutogenCharacters);

packages/@css-blocks/core/src/BlockParser/features/add-preset-selectors.ts

@@ -5,6 +5,7 @@ import { Block } from "../../BlockTree";
 import { Configuration } from "../../configuration";
 import { CssBlockError } from "../../errors";
 import { sourceRange } from "../../SourceLocation";
+import { getStyleTargets } from "../block-intermediates";
 import { stripQuotes } from "../utils";
 
 /**
@@ -26,11 +27,9 @@ import { stripQuotes } from "../utils";
 export function addPresetSelectors(configuration: Configuration, root: postcss.Root, block: Block, file: string) {
   // For each rule declared in the file...
   root.walkRules(rule => {
-    let foundDecl = false;
 
     // Find the block-class declaration...
     rule.walkDecls("block-class", decl => {
-      foundDecl = true;
       const val = stripQuotes(decl.value);
 
       // Test that this actually could be a class name.
@@ -44,21 +43,29 @@ export function addPresetSelectors(configuration: Configuration, root: postcss.R
       }
 
       // And add its value to the corresponding BlockClass node.
-      rule.selectors.forEach(sel => {
-        const node = block.find(sel);
-        if (!node) {
+      const parsedSelectors = block.getParsedSelectors(rule);
+      parsedSelectors.forEach(sel => {
+        const styleTarget = getStyleTargets(block, sel.key);
+        if (styleTarget.blockAttrs.length > 0) {
+          styleTarget.blockAttrs[0].setPresetClassName(val);
+        } else if (styleTarget.blockClasses.length > 0) {
+          styleTarget.blockClasses[0].setPresetClassName(val);
+        } else {
           throw new Error(`Couldn\'t find block class corresponding to selector ${sel}. This shouldn't happen.`);
         }
-        node.setPresetClassName(val);
       });
     });
+  });
 
-    // If we didn't find block-class declared, we should error.
-    if (!foundDecl) {
+  // At this point, every style node should have a fixed block-class.
+  block.all(true).forEach(styleNode => {
+    if (!styleNode.presetCssClass) {
       block.addError(
         new CssBlockError(
-          `Definition file rule ${rule.selectors} is missing a 'block-class' declaration`,
-          sourceRange(configuration, root, file, rule),
+          `Style node ${styleNode.asSource()} doesn't have a preset block class after parsing definition file. You may need to declare this style node in the definition file.`,
+          {
+            filename: file,
+          },
         ),
       );
     }

packages/@css-blocks/core/src/BlockParser/features/discover-guid.ts

@@ -6,7 +6,7 @@ import * as errors from "../../errors";
 import { sourceRange } from "../../SourceLocation";
 import { stripQuotes } from "../utils";
 
-export async function discoverGuid(configuration: Configuration, root: postcss.Root, file: string, isDfnFile = false, expectedId?: string): Promise<string | undefined> {
+export function discoverGuid(configuration: Configuration, root: postcss.Root, file: string, isDfnFile = false, expectedId?: string): string | undefined {
   let blockId: string | undefined;
   let scopeNode: postcss.Rule | undefined;
 
@@ -22,7 +22,7 @@ export async function discoverGuid(configuration: Configuration, root: postcss.R
       blockId = stripQuotes(decl.value.trim());
       // We don't have to expect an ID was declared in the header comment of the Compiled CSS
       // file, but we need to hard error if it's a mismatch.
-      if (expectedId && decl.value !== expectedId) {
+      if (expectedId && blockId !== expectedId) {
         throw new errors.InvalidBlockSyntax(
           `Expected block-id property in definition data to match header in Compiled CSS.`,
           sourceRange(configuration, root, file, decl),

packages/@css-blocks/core/src/BlockSyntax/BlockSyntax.ts

@@ -11,17 +11,20 @@ export const EXTENDS = "extends";
 export const IMPLEMENTS = "implements";
 export const BLOCK_NAME = "block-name";
 export const BLOCK_ID = "block-id";
+export const BLOCK_INTERFACE_INDEX = "block-interface-index";
+export const BLOCK_CLASS = "block-class";
 export const BLOCK_ALIAS = "block-alias";
 export const COMPOSES = "composes";
-export const BLOCK_PROP_NAMES = new Set([BLOCK_NAME, BLOCK_ALIAS, EXTENDS, IMPLEMENTS, COMPOSES]);
-export const BLOCK_PROP_NAMES_RE = /^(extends|implements|block-name|composes|block-alias)$/;
+export const BLOCK_PROP_NAMES = new Set([BLOCK_ID, BLOCK_NAME, BLOCK_CLASS, BLOCK_ALIAS, EXTENDS, IMPLEMENTS, COMPOSES, BLOCK_INTERFACE_INDEX]);
+export const BLOCK_PROP_NAMES_RE = /^(extends|implements|block-name|block-id|block-interface-index|block-class|composes|block-alias)$/;
 
 // At Rules
 export const BLOCK_DEBUG = "block-debug";
 export const BLOCK_GLOBAL = "block-global";
 export const BLOCK_IMPORT = "block";
 export const BLOCK_EXPORT = "export";
-export const BLOCK_AT_RULES = new Set([BLOCK_DEBUG, BLOCK_GLOBAL, BLOCK_IMPORT, BLOCK_EXPORT]);
+export const BLOCK_SYNTAX_VERSION = "block-syntax-version";
+export const BLOCK_AT_RULES = new Set([BLOCK_DEBUG, BLOCK_GLOBAL, BLOCK_IMPORT, BLOCK_EXPORT, BLOCK_SYNTAX_VERSION]);
 
 // Prop Values
 // TODO: Flesh out prop value parser APIs and actually use them.

packages/@css-blocks/core/src/BlockTree/AttrValue.ts

@@ -96,8 +96,8 @@ export class AttrValue extends Style<AttrValue, Block, Attribute, never> {
   }
 
   public cssClass(config: ResolvedConfiguration, reservedClassNames: Set<string>): string {
-    if (this.presetSelector) {
-      return this.presetSelector;
+    if (this.presetCssClass) {
+      return this.presetCssClass;
     }
     switch (config.outputMode) {
       case OutputMode.BEM:

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

@@ -442,7 +442,7 @@ export class Block
    * @param shallow - Pass true to exclude inherited objects.
    * @returns Collection of Styles objects, organized by preset selector value.
    */
-  compiledClassesMap(shallow?: boolean): ObjectDictionary<Styles> {
+  presetClassesMap(shallow?: boolean): ObjectDictionary<Styles> {
     const result = {};
     const all = this.all(shallow);
 

packages/@css-blocks/core/src/BlockTree/BlockClass.ts

@@ -205,8 +205,8 @@ export class BlockClass extends Style<BlockClass, Block, Block, Attribute> {
    * @returns String representing output class.
    */
   public cssClass(config: ResolvedConfiguration, reservedClassNames: Set<string>): string {
-    if (this.presetSelector) {
-      return this.presetSelector;
+    if (this.presetCssClass) {
+      return this.presetCssClass;
     }
     switch (config.outputMode) {
       case OutputMode.BEM:

packages/@css-blocks/core/src/BlockTree/Style.ts

@@ -36,7 +36,7 @@ export abstract class Style<
    * The preset selector for this particular class node. Set this if you're
    * loading in a definition file and `block-class` is set.
    */
-  protected presetSelector: string | undefined;
+  public presetCssClass: string | undefined;
 
   /**
    * Save name, parent container, and create the PropertyContainer for this data object.
@@ -52,15 +52,6 @@ export abstract class Style<
    */
   public abstract cssClass(config: ResolvedConfiguration, reservedClassNames: Set<string>): string;
 
-  /**
-   * The preset selector string for this style node. This is set if a specific
-   * class name was specified using a block-class declaration. This is only
-   * relevant to definition files.
-   */
-  get presetCssClass(): string | undefined {
-    return this.presetSelector;
-  }
-
   /**
    * Return the source selector this `Style` was read from.
    * @param scope  Optional scope to resolve this name relative to. If `true`, return the Block name instead of `:scope`. If a Block object, return with the local name instead of `:scope`.
@@ -118,7 +109,7 @@ export abstract class Style<
    * @param selector - The class name to set.
    */
   public setPresetClassName(selector: string): void {
-    this.presetSelector = selector;
+    this.presetCssClass = selector;
   }
 
   /**

packages/@css-blocks/core/src/PrecompiledDefinitions/block-syntax-version.ts

@@ -4,7 +4,7 @@ import { ImportedCompiledCssFile } from "../importing";
 /**
  * A regex to find the block-syntax-version annotation in a definition file.
  */
-const REGEXP_BLOCK_SYNTAX_VERSION = /^@block-syntax-version (\d+);/;
+const REGEXP_BLOCK_SYNTAX_VERSION = /^\s*@block-syntax-version ([^;]+);/m;
 
 /**
  * The earliest block-syntax-version supported by CSS Blocks. If this has been
@@ -33,19 +33,18 @@ export function determineBlockSyntaxVersion(file: ImportedCompiledCssFile): numb
   const versionLookupResult = dfnContents.match(REGEXP_BLOCK_SYNTAX_VERSION);
 
   if (!versionLookupResult) {
-    throw new CssBlockError("Unable to process definition file because the file is missing a block-syntax-version declaration.", {
+    throw new CssBlockError("Unable to process definition file because the file is missing a block-syntax-version declaration or it is malformed.", {
       filename: dfnId,
     });
   }
 
-  try {
-    const version = parseInt(versionLookupResult[1], 10);
-    return version;
-  } catch {
-    throw new CssBlockError("Unable to process definition file because the declared block-syntax version isn't a number.", {
+  const version = parseInt(versionLookupResult[1], 10);
+  if (isNaN(version)) {
+    throw new CssBlockError("Unable to process definition file because the declared block-syntax-version isn't a number.", {
       filename: dfnId,
     });
   }
+  return version;
 }
 
 /**

packages/@css-blocks/core/src/importing/BaseImporter.ts

@@ -75,13 +75,13 @@ export abstract class BaseImporter implements Importer {
     // Determine start/end indexes based on the regexp results above.
     const [headerFullMatch, blockId] = headerRegexpResult;
     const headerStartIndex = headerRegexpResult.index;
-    if (!headerStartIndex) {
+    if (typeof headerStartIndex === "undefined") {
       throw new Error("Unable to determine start location of regexp result.");
     }
     const headerEndIndex = headerStartIndex + headerFullMatch.length;
     const [footerFullMatch] = footerRegexpResult;
     const footerStartIndex = footerRegexpResult.index;
-    if (!footerStartIndex) {
+    if (typeof footerStartIndex === "undefined") {
       throw new Error("Unable to determine start location of regexp result.");
     }
     const footerEndIndex = footerStartIndex + footerFullMatch.length;
@@ -98,7 +98,7 @@ export abstract class BaseImporter implements Importer {
     // Parse out the URL, or embedded data, for the block definition.
     // The definition comment should be removed from the block's CSS contents.
     const definitionRegexpResult = fullBlockContents.match(REGEXP_COMMENT_DEFINITION_REF);
-    if (!definitionRegexpResult) {
+    if (definitionRegexpResult === null) {
       throw new Error("Unable to find definition URL in compiled CSS. This comment must be declared between the header and footer CSS Blocks comments.");
     }
     const [definitionFullMatch, definitionUrl] = definitionRegexpResult;