Skip to content

Commit 3912811

Browse files
author
Timothy Lindvall
committed
feat: Use incoming GUIDs. Ensure uniqueness.
- Update BlockParser to eagerly discover the block's GUID (if a definition file) and provide that GUID to the Block constructor. - Add check to Block constructor to validate that the GUID is valid. Also adds an attempt to recover if the GUID that was generated happens to match a previous entry.
1 parent a7384c8 commit 3912811

File tree

6 files changed

+102
-59
lines changed

6 files changed

+102
-59
lines changed

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

+5-6
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,12 @@ import { Options, ResolvedConfiguration, resolveConfiguration } from "../configu
66
import { FileIdentifier } from "../importing";
77

88
import { assertBlockClassDeclared } from "./features/assert-block-class-declared";
9-
import { assertBlockIdsMatch } from "./features/assert-block-ids-match";
109
import { assertForeignGlobalAttribute } from "./features/assert-foreign-global-attribute";
1110
import { composeBlock } from "./features/composes-block";
1211
import { constructBlock } from "./features/construct-block";
1312
import { disallowDefinitionRules } from "./features/disallow-dfn-rules";
1413
import { disallowImportant } from "./features/disallow-important";
14+
import { discoverGuid } from "./features/discover-guid";
1515
import { discoverName } from "./features/discover-name";
1616
import { exportBlocks } from "./features/export-blocks";
1717
import { extendBlock } from "./features/extend-block";
@@ -76,15 +76,14 @@ export class BlockParser {
7676
// Discover the block's preferred name.
7777
name = await discoverName(configuration, root, sourceFile, isDfnFile, name);
7878

79+
// Discover the block's GUID, if it's a definition file.
80+
const guid = await discoverGuid(configuration, root, sourceFile, isDfnFile, expectedId);
81+
7982
// Create our new Block object and save reference to the raw AST.
80-
let block = new Block(name, identifier, root);
83+
let block = new Block(name, identifier, root, guid);
8184

8285
if (isDfnFile) {
8386
// Rules only checked when parsing definition files...
84-
// Assert that the block-id rule in :scope is declared and matches
85-
// header comment in Compiled CSS.
86-
debug(" - Assert Block IDs Match");
87-
await assertBlockIdsMatch(block, configuration, root, sourceFile, expectedId);
8887
debug(" - Assert Block Class Declared");
8988
await assertBlockClassDeclared(block, configuration, root, sourceFile);
9089
} else {

packages/@css-blocks/core/src/BlockParser/features/assert-block-ids-match.ts

-50
This file was deleted.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { postcss } from "opticss";
2+
3+
import { BLOCK_ID } from "../../BlockSyntax";
4+
import { Configuration } from "../../configuration";
5+
import * as errors from "../../errors";
6+
import { sourceRange } from "../../SourceLocation";
7+
import { stripQuotes } from "../utils";
8+
9+
export async function discoverGuid(configuration: Configuration, root: postcss.Root, file: string, isDfnFile = false, expectedId?: string): Promise<string | undefined> {
10+
let blockId: string | undefined;
11+
let scopeNode: postcss.Rule | undefined;
12+
13+
// Only definition files can have GUIDs declared. It's not a critical failure if it's
14+
// defined... we'll check for this and complain about it later.
15+
if (!isDfnFile) {
16+
return;
17+
}
18+
19+
root.walkRules(":scope", (rule) => {
20+
scopeNode = rule;
21+
rule.walkDecls(BLOCK_ID, (decl) => {
22+
blockId = stripQuotes(decl.value.trim());
23+
// We don't have to expect an ID was declared in the header comment of the Compiled CSS
24+
// file, but we need to hard error if it's a mismatch.
25+
if (expectedId && decl.value !== expectedId) {
26+
throw new errors.InvalidBlockSyntax(
27+
`Expected block-id property in definition data to match header in Compiled CSS.`,
28+
sourceRange(configuration, root, file, decl),
29+
);
30+
}
31+
});
32+
});
33+
34+
// It's a critical failure if we can't find a block ID in the definition file, because
35+
// without it we can't reason about the Compiled CSS linked to this definition.
36+
if (!blockId) {
37+
if (scopeNode) {
38+
throw new errors.InvalidBlockSyntax(
39+
`Expected block-id to be declared in definition's :scope rule.`,
40+
sourceRange(configuration, root, file, scopeNode),
41+
);
42+
} else {
43+
throw new errors.InvalidBlockSyntax(
44+
`Expected block-id to be declared in definition's :scope rule.`,
45+
{
46+
filename: file,
47+
},
48+
);
49+
}
50+
}
51+
52+
return blockId;
53+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ export const CLASS_NAME_IDENT = new RegExp(regexpu("^(-?(?:\\\\.|[A-Za-z_\\u{008
1010
export const EXTENDS = "extends";
1111
export const IMPLEMENTS = "implements";
1212
export const BLOCK_NAME = "block-name";
13+
export const BLOCK_ID = "block-id";
1314
export const BLOCK_ALIAS = "block-alias";
1415
export const COMPOSES = "composes";
1516
export const BLOCK_PROP_NAMES = new Set([BLOCK_NAME, BLOCK_ALIAS, EXTENDS, IMPLEMENTS, COMPOSES]);

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

+42-2
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ function gen_guid(identifier: string): string {
4545
export class Block
4646
extends Inheritable<Block, Block, never, BlockClass> {
4747

48+
public static GUIDS_USED = new Map<string, string[]>();
49+
4850
private _blockReferences: ObjectDictionary<Block> = {};
4951
private _blockReferencesReverseLookup: Map<Block, string> = new Map();
5052
private _blockExports: ObjectDictionary<Block> = {};
@@ -66,13 +68,51 @@ export class Block
6668
public readonly rootClass: BlockClass;
6769
public stylesheet: postcss.Root | undefined;
6870

69-
constructor(name: string, identifier: FileIdentifier, stylesheet?: postcss.Root) {
71+
constructor(name: string, identifier: FileIdentifier, stylesheet?: postcss.Root, incomingGuid?: string) {
7072
super(name);
7173
this._identifier = identifier;
7274
this._dependencies = new Set<string>();
7375
this.rootClass = new BlockClass(ROOT_CLASS, this);
7476
this.stylesheet = stylesheet;
75-
this.guid = gen_guid(identifier);
77+
78+
// If we found a GUID from the :scope rule, use that. Otherwise, generate one.
79+
let guid = incomingGuid || gen_guid(identifier);
80+
81+
// We insist that each block have a unique GUID. In the event that we've somehow
82+
// ended up with two blocks that have the same GUID, our options depend on whether
83+
// we got a preset GUID passed in or not.
84+
if (Block.GUIDS_USED.has(guid)) {
85+
if (incomingGuid) {
86+
// If the GUID was already generated prior to creating the block, we have to
87+
// bail out. (There's likely Compiled CSS that depends on that GUID.)
88+
throw new CssBlockError("Block uses a GUID that has already been used!", {
89+
filename: identifier,
90+
});
91+
} else {
92+
// Ok, we autogenerated this. Let's append some gobbledygook to recover.
93+
const guidBaseList = Block.GUIDS_USED.get(guid);
94+
if (!guidBaseList) {
95+
// Ah crumbs. There should be a list but there isn't.
96+
throw new CssBlockError("Block uses a GUID that has already been used!", {
97+
filename: identifier,
98+
});
99+
}
100+
guid = `${guid}${guidBaseList.length}`;
101+
if (guidBaseList.includes(guid) || Block.GUIDS_USED.has(guid)) {
102+
// Ah crumbs. Our safety check to make sure the GUID really hasn't been used failed.
103+
throw new CssBlockError("Block uses a GUID that has already been used!", {
104+
filename: identifier,
105+
});
106+
}
107+
// Phew, okay, appending a numerical id works. Let's go with that.
108+
guidBaseList.push(guid);
109+
}
110+
} else {
111+
// Hey, this is unique! Cool, let's remember it in case we see it again later.
112+
Block.GUIDS_USED.set(guid, []);
113+
}
114+
this.guid = guid;
115+
76116
this.addClass(this.rootClass);
77117
}
78118

packages/@css-blocks/core/test/output-mode-test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export class BEMOutputMode extends BEMProcessor {
4848

4949
// Discover the generated GUID for this block.
5050
// It changes every time the process is killed.
51-
let uid = (css.match(/test-block_(.....)/) || [])[1];
51+
let uid = (css.match(/test-block_([^\s{]+)/) || [])[1];
5252

5353
assert.deepEqual(
5454
minify(css),

0 commit comments

Comments
 (0)