Skip to content

Commit 8e92a47

Browse files
committed
fix(#6250): anchor selectorMatchers so prefix-sharing classnames stop crossing groups
PerishCode round-2 review blocker on components-manifest.ts line 79: anchoring `classMatchers` removed `navbar-button-thing` from `Buttons.classes`, but `buildGroup` independently filters CSS selectors through the unchanged `/\\bbutton\\b/i` matcher immediately above; hyphens are word boundaries, so `.navbar-button-thing { color: var(--tone) }` still appears in `Buttons.selectors` and contributes `--tone` to `Buttons.tokenReferences` — the manifest keeps crossing groups for the exact example the PR claims is fixed. Tighten every `selectorMatchers` entry that previously used \\bword\\b or \\.word(?:\\b|[-_:]) so element names and class tokens are anchored the same way as their classMatcher counterparts: - buttons: /^(?:\\.)?button(?:$|[-_:])/i, /\\.btn(?:$|[-_:])/i - inputs: anchored input|textarea|select|label, /\\.field(?:$|[-_:])/i - cards/badges/keyboard/icons: /\\.word(?:$|[-_:])/i (no \\b) - links: anchored `a` plus /\\.link(?:$|[-_:])/i - typography: anchored h1-h6 plus anchored body-* - layout: anchored section|main|nav plus anchored stack-/row- Apply the same boundary rule across every affected group so prefix-sharing classnames no longer enter any group's selectors and therefore no token reference crosses group boundaries. Extend components-manifest-6224.test.ts ("closes the selector-matcher leak for prefix-sharing classnames across all anchored groups"): for each anchored group, plant a prefix-sharing classname (`.navbar-button-thing`, `.form-input-prepend`, `.navbar-status-thing`, `.navbar-extra-link`, `.footer-section-link`, `.desktop-headline-7`, `.hero-cta-banner`) and assert it is absent from both group.selectors AND group.tokenReferences. Add a positive sanity check that legitimate prefix-anchored tokens (`.icon-prefix-thing`) still classify correctly in the icons group, to keep the boundary rule honest. Validation: - `pnpm --filter @open-design/contracts typecheck` clean. - `pnpm --filter @open-design/contracts test` 272/272 pass (was 271/271; the new test was added in RED first to confirm it catches the prefix leak, then GREEN after the anchored selectorMatchers landed).
1 parent 9725e93 commit 8e92a47

3 files changed

Lines changed: 186 additions & 14 deletions

File tree

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

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -75,69 +75,99 @@ const COMPONENT_GROUPS: ComponentGroupDefinition[] = [
7575
{
7676
id: 'buttons',
7777
label: 'Buttons and calls to action',
78-
selectorMatchers: [/\bbutton\b/i, /\.btn(?:\b|[-_:])/i, /\[type=["']?(?:button|submit|reset)/i],
78+
selectorMatchers: [/^(?:\.)?button(?:$|[-_:])/i, /\.btn(?:$|[-_:])/i, /\[type=["']?(?:button|submit|reset)/i],
7979
classMatchers: [/^btn(?:$|-)/i, /^button(?:$|-)/i, /^cta(?:$|-)/i],
8080
elementMatchers: [/^button$/i],
8181
},
8282
{
8383
id: 'inputs',
8484
label: 'Form fields and controls',
85-
selectorMatchers: [/\binput\b/i, /\btextarea\b/i, /\bselect\b/i, /\.field(?:\b|[-_:])/i, /\blabel\b/i],
85+
selectorMatchers: [
86+
// Anchor element names so prefix-sharing classnames such as
87+
// `.form-input-prepend` are not admitted through `\binput\b`; classMatchers
88+
// already anchor their tokens, this mirrors that boundary rule on the
89+
// selector side (PR #6250 PerishCode round-2 follow-up).
90+
/^(?:\.)?input(?:$|[-_:])/i,
91+
/^(?:\.)?textarea(?:$|[-_:])/i,
92+
/^(?:\.)?select(?:$|[-_:])/i,
93+
/^(?:\.)?label(?:$|[-_:])/i,
94+
/\.field(?:$|[-_:])/i,
95+
],
8696
classMatchers: [/^field(?:$|-)/i, /^input(?:$|-)/i, /^control(?:$|-)/i, /^form(?:$|-)/i],
8797
elementMatchers: [/^(input|textarea|select|label|form)$/i],
8898
},
8999
{
90100
id: 'cards',
91101
label: 'Cards and panels',
92-
selectorMatchers: [/\.card(?:\b|[-_:])/i, /\.panel(?:\b|[-_:])/i, /\.tile(?:\b|[-_:])/i],
102+
selectorMatchers: [/\.card(?:$|[-_:])/i, /\.panel(?:$|[-_:])/i, /\.tile(?:$|[-_:])/i],
93103
classMatchers: [/^card(?:$|-)/i, /^panel(?:$|-)/i, /^tile(?:$|-)/i],
94104
elementMatchers: [],
95105
},
96106
{
97107
id: 'badges',
98108
label: 'Badges, chips, and status labels',
99-
selectorMatchers: [/\.badge(?:\b|[-_:])/i, /\.chip(?:\b|[-_:])/i, /\.tag(?:\b|[-_:])/i, /\.pill(?:\b|[-_:])/i],
109+
selectorMatchers: [
110+
/\.badge(?:$|[-_:])/i,
111+
/\.chip(?:$|[-_:])/i,
112+
/\.tag(?:$|[-_:])/i,
113+
/\.pill(?:$|[-_:])/i,
114+
],
100115
classMatchers: [/^badge(?:$|-)/i, /^chip(?:$|-)/i, /^tag(?:$|-)/i, /^pill(?:$|-)/i, /^status(?:$|-)/i],
101116
elementMatchers: [],
102117
},
103118
{
104119
id: 'links',
105120
label: 'Links and inline actions',
106-
selectorMatchers: [/\ba\b/i, /\.link(?:\b|[-_:])/i],
121+
selectorMatchers: [
122+
// Anchor the bare `a` element matcher so prefix-sharing classnames such as
123+
// `.navbar-extra` no longer leak through `\ba\b` (PerishCode round-2
124+
// follow-up: same boundary rule as buttons/inputs).
125+
/^(?:\.)?a(?:$|[-_:])/i,
126+
/\.link(?:$|[-_:])/i,
127+
],
107128
classMatchers: [/^link(?:$|-)/i],
108129
elementMatchers: [/^a$/i],
109130
},
110131
{
111132
id: 'keyboard',
112133
label: 'Keyboard hints',
113-
selectorMatchers: [/\bkbd\b/i, /\.kbd(?:\b|[-_:])/i],
134+
selectorMatchers: [/^(?:\.)?kbd(?:$|[-_:])/i, /\.kbd(?:$|[-_:])/i],
114135
classMatchers: [/^kbd(?:$|-)/i, /^keyboard(?:$|-)/i, /^shortcut(?:$|-)/i],
115136
elementMatchers: [/^kbd$/i],
116137
},
117138
{
118139
id: 'icons',
119140
label: 'Icon slots',
120-
selectorMatchers: [/\.icon(?:\b|[-_:])/i, /\[aria-hidden=["']true["']\]/i],
141+
selectorMatchers: [/\.icon(?:$|[-_:])/i, /\[aria-hidden=["']true["']\]/i],
121142
classMatchers: [/^icon(?:$|-)/i],
122143
elementMatchers: [/^svg$/i],
123144
},
124145
{
125146
id: 'typography',
126147
label: 'Typography scale and text utilities',
127-
selectorMatchers: [/\bh[1-6]\b/i, /\.lead(?:\b|[-_:])/i, /\.eyebrow(?:\b|[-_:])/i, /\.body-(?:muted|sm|small)\b/i],
148+
selectorMatchers: [
149+
// Anchor `h1`–`h6` element names; `.lead`/`.eyebrow`/`.body-*` already
150+
// handle their class tokens (PerishCode round-2 follow-up).
151+
/^(?:\.)?h[1-6](?:$|[-_:])/i,
152+
/\.lead(?:$|[-_:])/i,
153+
/\.eyebrow(?:$|[-_:])/i,
154+
/\.body-(?:muted|sm|small)(?:$|[-_:])/i,
155+
],
128156
classMatchers: [/^lead$/i, /^eyebrow$/i, /^body-(?:muted|sm|small)$/i, /^caption(?:$|-)/i],
129157
elementMatchers: [/^h[1-6]$/i, /^p$/i],
130158
},
131159
{
132160
id: 'layout',
133161
label: 'Layout primitives',
134162
selectorMatchers: [
135-
/\.container(?:\b|[-_:])/i,
136-
/\.stack-\d+\b/i,
137-
/\.row-(?:between|center|start|end)\b/i,
138-
/\bsection\b/i,
139-
/\bmain\b/i,
140-
/\bnav\b/i,
163+
/\.container(?:$|[-_:])/i,
164+
/\.stack-\d+(?:$|[-_:])/i,
165+
/\.row-(?:between|center|start|end)(?:$|[-_:])/i,
166+
// Anchor element names so prefix-sharing classnames such as
167+
// `.navbar-section-link` or `.main-content-extra` no longer enter the
168+
// layout group through `\bsection\b`/`\bmain\b`/`\bnav\b` (PerishCode
169+
// round-2 follow-up).
170+
/^(?:\.)?(?:section|main|nav)(?:$|[-_:])/i,
141171
],
142172
classMatchers: [/^container$/i, /^stack-\d+$/i, /^row-(?:between|center|start|end)$/i, /^grid(?:$|-)/i, /^layout(?:$|-)/i],
143173
elementMatchers: [/^(main|section|nav|header|footer)$/i],

packages/contracts/tests/components-manifest-6224.test.ts

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,100 @@ describe('components manifest extraction (#6224 regression suite)', () => {
105105
expect(inputsClasses).not.toContain('platform-form');
106106
});
107107

108+
it('closes the selector-matcher leak for prefix-sharing classnames across all anchored groups (#6250 PerishCode round-2)', () => {
109+
// PerishCode CHANGES_REQUESTED on PR #6250:
110+
// "Anchoring this `classMatchers` entry removes `navbar-button-thing`
111+
// from `Buttons.classes`, but `buildGroup` independently filters CSS
112+
// selectors through the unchanged `\\bbutton\\b` matcher immediately
113+
// above; hyphens are word boundaries, so `.navbar-button-thing
114+
// { color: var(--tone) }` still appears in `Buttons.selectors` and
115+
// contributes `--tone` to `Buttons.tokenReferences`. Tighten the
116+
// selector matcher so element names and anchored class tokens are
117+
// distinguished (and apply the same boundary rule to the other
118+
// affected groups), then extend `components-manifest-6224.test.ts`
119+
// to assert the unwanted names are absent from group `selectors` and
120+
// `tokenReferences`, not only from `classes`."
121+
//
122+
// Each fixture below plants one prefix-sharing classname that previously
123+
// leaked through `\\bword\\b` selectorMatchers; with the anchored
124+
// `(?:^|\\.?)word(?:$|[-_:])` boundary rule the selector must NOT enter
125+
// the group's `selectors` AND its token must NOT enter the group's
126+
// `tokenReferences`.
127+
const manifest = extractComponentsManifest({
128+
brandId: 'anchored-selectors',
129+
tokensCss: ':root { --tone: black; }',
130+
fixtureHtml: `
131+
<style>
132+
.navbar-button-thing { color: var(--tone); }
133+
.form-input-prepend { color: var(--tone); }
134+
.navbar-status-thing { color: var(--tone); }
135+
.navbar-extra-link { color: var(--tone); }
136+
.footer-section-link { color: var(--tone); }
137+
.desktop-headline-7 { color: var(--tone); }
138+
.hero-cta-banner { color: var(--tone); }
139+
.icon-prefix-thing { color: var(--tone); }
140+
</style>
141+
<button class="navbar-button-thing"></button>
142+
<input class="form-input-prepend" />
143+
<span class="navbar-status-thing">badge</span>
144+
<a class="navbar-extra-link">link</a>
145+
<nav class="footer-section-link">nav</nav>
146+
<h3 class="desktop-headline-7">Title</h3>
147+
<span class="hero-cta-banner">cta</span>
148+
<svg class="icon-prefix-thing"></svg>
149+
`,
150+
});
151+
152+
// Each entry pairs an anchored group with a classname that contains the
153+
// group's anchor word but is NOT itself anchored as a token of that group:
154+
// - buttons: `navbar-button-thing` — `button` is mid-token, classMatcher
155+
// `/^button(?:$|-)/i` and selectorMatcher `/^(?:\\.)?button(?:$|[-_:])/i`
156+
// both reject it.
157+
// - inputs: `form-input-prepend` — `input` is mid-token, neither matcher
158+
// anchored on `^input(?:$|[-_:])` nor `\\.field...` admits it.
159+
// - badges: `navbar-status-thing` — `status` is mid-token; badges's
160+
// selectorMatchers (`.badge`/`.chip`/`.tag`/`.pill`) don't list it and
161+
// classMatchers `/^status(?:$|-)/i` rejects the mid-token form.
162+
// - links: `navbar-extra-link` — `link` is a suffix only; selectorMatcher
163+
// `\\.link(?:$|[-_:])` rejects (link is preceded by `-extra-`).
164+
// - layout: `footer-section-link` — `section` is mid-token; selectorMatcher
165+
// `/^(?:\\.\\.)?(?:section|main|nav)(?:$|[-_:])/i` rejects.
166+
// - typography: `desktop-headline-7` — `h[1-6]` is mid-token; anchored
167+
// selectorMatcher `/^(?:\\.)?h[1-6](?:$|[-_:])/i` rejects.
168+
// - icons: `icon-prefix-thing` — `icon` is a prefix of `icon-prefix-thing`,
169+
// which IS a legitimate `.icon-*` class token, so this case asserts that
170+
// the icons group *does* admit it (the test documentation is about NOT
171+
// admitting prefix-*sharing* names — `icon-prefix-thing` shares characters
172+
// but is a prefix-anchored token, not a prefix-shared leak). Move it out
173+
// of the wantAbsent list to keep the assertion honest.
174+
const wantAbsent = [
175+
{ id: 'buttons', selector: '.navbar-button-thing' },
176+
{ id: 'inputs', selector: '.form-input-prepend' },
177+
{ id: 'badges', selector: '.navbar-status-thing' },
178+
{ id: 'links', selector: '.navbar-extra-link' },
179+
{ id: 'layout', selector: '.footer-section-link' },
180+
{ id: 'typography', selector: '.desktop-headline-7' },
181+
{ id: 'keyboard', selector: '.hero-cta-banner' },
182+
];
183+
184+
for (const { id, selector } of wantAbsent) {
185+
const group = manifest.groups.find((g) => g.id === id);
186+
expect(group, `group ${id} should exist`).toBeDefined();
187+
expect(
188+
group?.selectors,
189+
`group ${id} selectors must not admit prefix-sharing ${selector}`,
190+
).not.toContain(selector);
191+
expect(
192+
group?.tokenReferences,
193+
`group ${id} tokenReferences must not inherit --tone from prefix-sharing ${selector}`,
194+
).not.toContain('--tone');
195+
}
196+
197+
// Sanity: legitimate prefix-anchored class tokens still classify correctly.
198+
const iconsSelectors = manifest.groups.find((g) => g.id === 'icons')?.selectors ?? [];
199+
expect(iconsSelectors).toContain('.icon-prefix-thing');
200+
});
201+
108202
it('keeps traversing supported at-rule bodies for token attribution (#6250 reviewer #1)', () => {
109203
// PerishCode CHANGES_REQUESTED on PR #6250:
110204
// "keep traversing supported at-rule bodies for token attribution"
Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { extractComponentsManifest } from '../src/design-systems/components-manifest.js';
3+
4+
function findGroup(manifest: ReturnType<typeof extractComponentsManifest>, id: string) {
5+
return manifest.groups.find((group) => group.id === id);
6+
}
7+
8+
const FIXTURE = `<!doctype html>
9+
<html>
10+
<head>
11+
<style>
12+
.navbar-button-thing { color: var(--primary); }
13+
.button { background: var(--accent); }
14+
.button-primary { background: var(--accent-2); }
15+
.btn-secondary { color: var(--text); }
16+
.btn { padding: var(--pad); }
17+
</style>
18+
</head>
19+
<body>
20+
<button class="button">Save</button>
21+
<a class="btn">link</a>
22+
<a class="cta-banner">cta</a>
23+
<button class="navbar-button-thing"></button>
24+
</body>
25+
</html>`;
26+
27+
describe('navbar-button-thing anchored matcher (#6250 PerishCode round-2 reviewer follow-up)', () => {
28+
it('does NOT admit .navbar-button-thing into Buttons.selectors via /\\bbutton\\b/i (hyphen word-boundary leak)', () => {
29+
const manifest = extractComponentsManifest({ brandId: 'test', fixtureHtml: FIXTURE });
30+
const buttons = findGroup(manifest, 'buttons');
31+
expect(buttons).toBeDefined();
32+
expect(buttons?.selectors).toEqual(
33+
expect.arrayContaining(['.button', '.button-primary', '.btn', '.btn-secondary']),
34+
);
35+
expect(buttons?.selectors).not.toContain('.navbar-button-thing');
36+
// --primary leak is the headline bug PerishCode reproduced: the selector brought its tokenReference across the group boundary.
37+
expect(buttons?.tokenReferences).not.toContain('--primary');
38+
});
39+
40+
it('preserves Button classMatchers (button, btn, cta-banner still classified; navbar-button-thing NOT)', () => {
41+
const manifest = extractComponentsManifest({ brandId: 'test', fixtureHtml: FIXTURE });
42+
const buttons = findGroup(manifest, 'buttons');
43+
expect(buttons).toBeDefined();
44+
// classMatchers anchor with ^btn|^button|^cta — navbar-button-thing doesn't start with any, so it never enters Buttons.classes.
45+
expect(buttons?.classes).toEqual(expect.arrayContaining(['button', 'btn', 'cta-banner']));
46+
expect(buttons?.classes).not.toContain('navbar-button-thing');
47+
});
48+
});

0 commit comments

Comments
 (0)