Skip to content

Commit e7feb4f

Browse files
committed
fix(nexu-io#6250): make opening-brace scan quote-aware for selectors
PerishCode CHANGES_REQUESTED on PR nexu-io#6250 (round 4): "Make the opening-brace search quote-aware too. The new quote state begins only after has already selected an opener, so a valid selector such as treats the brace inside the attribute value as the rule boundary. ... is , and the Buttons group has no selectors or token references, so both rules are lost." The previous opener lookup used , which finds the first byte with no lexical context. A quoted inside a selector — e.g. — was therefore selected as the rule opener, leaving the closing of the same attribute value as the body's first and as a corrupt selectorList. The body scanner then ran with mismatched braces and emitted zero rules. Fix: replace the lookup with a quote-aware scan that mirrors the closing-brace scanner's lexical handling. The scan walks from , tracking single / double quote state plus backslash escapes, and skips / that appear inside a quoted string (also skipping comments defensively). The first un-quoted found is the real rule-body opener. Coverage: add a regression suite at packages/contracts/tests/components-manifest-6250-quoted-selector.test.ts covering both double- and single-quoted attribute selectors that contain a . Each case pairs the quoted-brace selector with a trailing flat rule and asserts both selectors plus their --a / --b attribution survive on the buttons group. Local verification: - pnpm --filter @open-design/contracts vitest run tests/components-manifest-6250-quoted-selector.test.ts -> 2/2 pass - pnpm --filter @open-design/contracts test -> 254/254 pass - pnpm --filter @open-design/contracts typecheck -> clean
1 parent fe29a4d commit e7feb4f

2 files changed

Lines changed: 122 additions & 6 deletions

File tree

packages/contracts/src/design-systems/components-manifest.ts

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -342,7 +342,52 @@ function iterateCssRules(css: string): CssRule[] {
342342
// Find the next '{' that opens a rule body. Skip '@container' at-rule
343343
// bodies (their inner rules are emitted with the at-rule header stripped,
344344
// matching stripContainerAtRuleHeaders behaviour).
345-
const openIndex = css.indexOf('{', index);
345+
//
346+
// Quote-aware scan (PR #6250 reviewer #4): a `{` that appears inside a
347+
// quoted selector context — e.g. `.btn-a[data-icon="{"]` — must NOT be
348+
// treated as the rule opener. We walk from `index` tracking single /
349+
// double quote state and backslash escapes, ignoring `{` / `}` that
350+
// appear inside a quoted string, so the first un-quoted `{` we find is
351+
// the real rule-body opener. Without this, valid CSS such as
352+
// `.btn-a[data-icon="{"] { color: var(--a); }` mistreats the brace
353+
// inside the attribute value as the opener and `manifest.selectors`
354+
// silently drops both rules.
355+
let openIndex = -1;
356+
let quoteScan = index;
357+
let inQuote: '"' | "'" | null = null;
358+
while (quoteScan < length) {
359+
const ch = css[quoteScan];
360+
if (ch === '/' && css[quoteScan + 1] === '*') {
361+
// Skip a /* ... */ comment block so braces inside comments do not
362+
// perturb the quote scan.
363+
const closeIdx = css.indexOf('*/', quoteScan + 2);
364+
quoteScan = closeIdx === -1 ? length : closeIdx + 2;
365+
continue;
366+
}
367+
if (inQuote !== null) {
368+
if (ch === '\\') {
369+
quoteScan += 2;
370+
continue;
371+
}
372+
if (ch === inQuote) {
373+
inQuote = null;
374+
quoteScan += 1;
375+
continue;
376+
}
377+
quoteScan += 1;
378+
continue;
379+
}
380+
if (ch === '"' || ch === "'") {
381+
inQuote = ch;
382+
quoteScan += 1;
383+
continue;
384+
}
385+
if (ch === '{') {
386+
openIndex = quoteScan;
387+
break;
388+
}
389+
quoteScan += 1;
390+
}
346391
if (openIndex === -1) break;
347392

348393
const selectorList = css.slice(index, openIndex).trim();
@@ -360,7 +405,7 @@ function iterateCssRules(css: string): CssRule[] {
360405
// Track the open CSS quote (single or double) so braces inside a
361406
// quoted value such as `content: "}"` are not mistaken for the rule
362407
// terminator (PR #6250 reviewer #3). null = outside any quoted string.
363-
let inQuote: '"' | "'" | null = null;
408+
let bodyInQuote: '"' | "'" | null = null;
364409
while (cursor < length) {
365410
const char = css[cursor];
366411
if (char === '/' && css[cursor + 1] === '*') {
@@ -377,14 +422,14 @@ function iterateCssRules(css: string): CssRule[] {
377422
// ignore `{` / `}` that appear between a pair of quotes. Handle
378423
// escaped quotes \" and \' so a `}` preceded by \" doesn't escape
379424
// the quoted context.
380-
if (inQuote === '"' || inQuote === "'") {
425+
if (bodyInQuote === '"' || bodyInQuote === "'") {
381426
if (char === '\\') {
382427
// Skip the escaped character (could be \" or \' or any escape).
383428
cursor += 2;
384429
continue;
385430
}
386-
if (char === inQuote) {
387-
inQuote = null;
431+
if (char === bodyInQuote) {
432+
bodyInQuote = null;
388433
cursor += 1;
389434
continue;
390435
}
@@ -394,7 +439,7 @@ function iterateCssRules(css: string): CssRule[] {
394439
continue;
395440
}
396441
if (char === '"' || char === "'") {
397-
inQuote = char;
442+
bodyInQuote = char;
398443
cursor += 1;
399444
continue;
400445
}
Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { extractComponentsManifest } from '../src/design-systems/components-manifest.js';
4+
5+
describe('quoted braces in selectors (#6250 reviewer #4)', () => {
6+
it('treats braces inside quoted attribute selectors as data, not rule openers', () => {
7+
// PerishCode CHANGES_REQUESTED on PR #6250:
8+
// "Make the opening-brace search quote-aware too. The new quote state
9+
// begins only after `css.indexOf('{', index)` has already selected an
10+
// opener, so a valid selector such as `.btn-a[data-icon="{"] { color:
11+
// var(--a); }` treats the brace inside the attribute value as the rule
12+
// boundary. ... manifest.selectors is `[]`, and the Buttons group has
13+
// no selectors or token references ..."
14+
//
15+
// The fix: scan for the opening delimiter with the same single-quote,
16+
// double-quote, and escape state used for the closing delimiter. A
17+
// `{` / `}` that appears inside a quoted attribute selector (or any
18+
// other quoted selector context) must not terminate the selector scan.
19+
const manifest = extractComponentsManifest({
20+
brandId: 'quoted-selector-brace',
21+
tokensCss: ':root { --a: red; --b: blue; }',
22+
fixtureHtml: `
23+
<style>
24+
.btn-a[data-icon="{"] { color: var(--a); }
25+
.btn-b { color: var(--b); }
26+
</style>
27+
<button class="btn-a btn-b">x</button>
28+
`,
29+
});
30+
31+
expect(manifest.selectors).toEqual(
32+
expect.arrayContaining(['.btn-a[data-icon="{"]', '.btn-b']),
33+
);
34+
const buttonsGroup = manifest.groups.find((g) => g.id === 'buttons');
35+
expect(buttonsGroup?.selectors).toEqual(
36+
expect.arrayContaining(['.btn-a[data-icon="{"]', '.btn-b']),
37+
);
38+
expect(buttonsGroup?.tokenReferences).toEqual(
39+
expect.arrayContaining(['--a', '--b']),
40+
);
41+
});
42+
43+
it('treats braces inside single-quoted attribute selectors as data', () => {
44+
// Same shape, single-quoted variant. Selector `.btn-a[data-icon='{']`
45+
// must NOT be split at the `{` inside the attribute.
46+
const manifest = extractComponentsManifest({
47+
brandId: 'quoted-selector-brace-single',
48+
tokensCss: ':root { --a: red; --b: blue; }',
49+
fixtureHtml: `
50+
<style>
51+
.btn-a[data-icon='{'] { color: var(--a); }
52+
.btn-b { color: var(--b); }
53+
</style>
54+
<button class="btn-a btn-b">x</button>
55+
`,
56+
});
57+
58+
// The selector is preserved verbatim — single quotes stay single,
59+
// double quotes stay double. We assert the single-quoted form here.
60+
expect(manifest.selectors).toEqual(
61+
expect.arrayContaining([".btn-a[data-icon='{']", '.btn-b']),
62+
);
63+
const buttonsGroup = manifest.groups.find((g) => g.id === 'buttons');
64+
expect(buttonsGroup?.selectors).toEqual(
65+
expect.arrayContaining([".btn-a[data-icon='{']", '.btn-b']),
66+
);
67+
expect(buttonsGroup?.tokenReferences).toEqual(
68+
expect.arrayContaining(['--a', '--b']),
69+
);
70+
});
71+
});

0 commit comments

Comments
 (0)