Skip to content

Commit 00508af

Browse files
himerusclaude
andcommitted
chore: resolve merge conflict in benchmark fixture
Accept HEAD timestamps after dev merge. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2 parents 36abde2 + 0a30bac commit 00508af

File tree

7 files changed

+225
-26
lines changed

7 files changed

+225
-26
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'helixir': patch
3+
'@helixir/core': patch
4+
---
5+
6+
fix: buildCssSnippet self-referential var() bug, add slot styling guidance and font inheritance warnings
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'helixir': minor
3+
'@helixir/core': minor
4+
---
5+
6+
feat: make suggest_fix token suggestions library-agnostic with optional tokenPrefix parameter

packages/core/src/handlers/styling-diagnostics.ts

Lines changed: 51 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,16 @@ export function buildAntiPatterns(meta: ComponentMetadata): AntiPatternWarning[]
188188
});
189189
}
190190

191+
// Warn about font vs layout inheritance through slots
192+
if (meta.slots.length > 0) {
193+
warnings.push({
194+
pattern: `${tag}::slotted(div) { margin: 10px; } /* expecting layout to inherit */`,
195+
explanation:
196+
'Slotted content inherits font styles (color, font-size, line-height) from the shadow DOM, but layout properties (margin, padding, display, width) must be set in light DOM CSS — they do not inherit through the shadow boundary.',
197+
correctApproach: `Style layout in light DOM CSS: ${tag} > div { margin: 10px; }. Font properties like color and font-size will inherit from the component's shadow DOM automatically.`,
198+
});
199+
}
200+
191201
return warnings;
192202
}
193203

@@ -200,13 +210,14 @@ export function buildCssSnippet(meta: ComponentMetadata): string {
200210
const lines: string[] = [];
201211
const tag = meta.tagName;
202212

203-
// Token customization section
213+
// Token customization section — show how to OVERRIDE custom properties
204214
if (meta.cssProperties.length > 0) {
205-
lines.push(`/* Token customization */`);
215+
lines.push(`/* Token customization — override on the component host */`);
206216
lines.push(`${tag} {`);
207217
for (const prop of meta.cssProperties.slice(0, 5)) {
208-
const defaultVal = prop.description ? `/* ${prop.description} */` : '';
209-
lines.push(` ${prop.name}: var(${prop.name}) ${defaultVal};`.trimEnd());
218+
const value = prop.default ?? guessDefaultValue(prop.name);
219+
const comment = prop.description ? ` /* ${prop.description} */` : '';
220+
lines.push(` ${prop.name}: ${value};${comment}`);
210221
}
211222
lines.push(`}`);
212223
}
@@ -223,6 +234,25 @@ export function buildCssSnippet(meta: ComponentMetadata): string {
223234
}
224235
}
225236

237+
// Slot styling section — show how to style slotted content in light DOM
238+
if (meta.slots.length > 0) {
239+
lines.push('');
240+
lines.push(`/* Slot styling — target slotted elements in light DOM CSS */`);
241+
const hasDefaultSlot = meta.slots.some((s) => s.name === '');
242+
const namedSlots = meta.slots.filter((s) => s.name !== '');
243+
244+
if (hasDefaultSlot) {
245+
lines.push(`${tag} > * { /* styles for default slot content */ }`);
246+
}
247+
for (const slot of namedSlots.slice(0, 3)) {
248+
const desc = slot.description ? ` /* ${slot.description} */` : '';
249+
lines.push(`${tag} [slot="${slot.name}"] { ${desc.trim()} }`);
250+
}
251+
lines.push(`/* Note: slotted content inherits font styles (color, font-size)`);
252+
lines.push(` from the shadow DOM, but layout (margin, padding, display)`);
253+
lines.push(` must be set here in light DOM CSS. */`);
254+
}
255+
226256
if (lines.length === 0) {
227257
lines.push(`/* ${tag} exposes no CSS customization points in its CEM. */`);
228258
lines.push(`/* Check the component documentation for styling options. */`);
@@ -231,6 +261,23 @@ export function buildCssSnippet(meta: ComponentMetadata): string {
231261
return lines.join('\n');
232262
}
233263

264+
/**
265+
* Guesses a sensible placeholder value based on CSS property name patterns.
266+
* Used when the CEM doesn't specify a default value.
267+
*/
268+
function guessDefaultValue(propName: string): string {
269+
const lower = propName.toLowerCase();
270+
if (/color|bg|background/.test(lower)) return '#value';
271+
if (/size|font/.test(lower)) return '1rem';
272+
if (/radius/.test(lower)) return '4px';
273+
if (/spacing|padding|margin|gap/.test(lower)) return '1rem';
274+
if (/shadow/.test(lower)) return '0 1px 2px rgba(0,0,0,.1)';
275+
if (/weight/.test(lower)) return '400';
276+
if (/width|height/.test(lower)) return 'auto';
277+
if (/opacity/.test(lower)) return '1';
278+
return '#value';
279+
}
280+
234281
// ─── Main Entry Point ────────────────────────────────────────────────────────
235282

236283
/**

packages/core/src/handlers/suggest-fix.ts

Lines changed: 36 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,26 +36,41 @@ export interface SuggestFixInput {
3636
memberName?: string;
3737
suggestedName?: string;
3838
eventName?: string;
39+
/** Optional token prefix from the component's library (e.g. '--hx-', '--fast-'). When provided, suggested tokens use this prefix instead of generic placeholders. */
40+
tokenPrefix?: string;
3941
}
4042

4143
// ─── Token heuristics ────────────────────────────────────────────────────────
4244

43-
const TOKEN_SUGGESTIONS: Record<string, string> = {
44-
'background-color': '--sl-color-neutral-0',
45-
background: '--sl-color-neutral-0',
46-
color: '--sl-color-neutral-900',
47-
'border-color': '--sl-color-neutral-300',
48-
'box-shadow': '--sl-shadow-medium',
49-
'font-family': '--sl-font-sans',
50-
'font-size': '--sl-font-size-medium',
51-
'border-radius': '--sl-border-radius-medium',
52-
padding: '--sl-spacing-medium',
53-
margin: '--sl-spacing-medium',
54-
gap: '--sl-spacing-small',
45+
/** Maps CSS properties to semantic token suffixes (library-agnostic). */
46+
const TOKEN_SUFFIXES: Record<string, string> = {
47+
'background-color': 'color-neutral-0',
48+
background: 'color-neutral-0',
49+
color: 'color-neutral-900',
50+
'border-color': 'color-neutral-300',
51+
'box-shadow': 'shadow-medium',
52+
'font-family': 'font-sans',
53+
'font-size': 'font-size-medium',
54+
'border-radius': 'border-radius-medium',
55+
padding: 'spacing-medium',
56+
margin: 'spacing-medium',
57+
gap: 'spacing-small',
5558
};
5659

57-
function suggestTokenForProperty(property: string): string {
58-
return TOKEN_SUGGESTIONS[property] ?? '--your-design-token';
60+
/**
61+
* Suggests a token name for a CSS property. When a tokenPrefix is provided,
62+
* generates a library-specific token (e.g. `--hx-color-neutral-0`).
63+
* Without a prefix, returns a generic placeholder.
64+
*/
65+
function suggestTokenForProperty(property: string, tokenPrefix?: string): string {
66+
const suffix = TOKEN_SUFFIXES[property];
67+
if (!suffix) {
68+
return tokenPrefix ? `${tokenPrefix}design-token` : '--your-design-token';
69+
}
70+
if (tokenPrefix) {
71+
return `${tokenPrefix}${suffix}`;
72+
}
73+
return `--your-${suffix}`;
5974
}
6075

6176
// ─── Shadow DOM fixes ────────────────────────────────────────────────────────
@@ -193,7 +208,7 @@ function fixShadowDom(input: SuggestFixInput): FixSuggestion {
193208
// ─── Token fallback fixes ────────────────────────────────────────────────────
194209

195210
function fixTokenFallback(input: SuggestFixInput): FixSuggestion {
196-
const { original, property } = input;
211+
const { original, property, tokenPrefix } = input;
197212

198213
if (input.issue === 'missing-fallback') {
199214
// Extract the var() call and add a sensible fallback
@@ -223,7 +238,7 @@ function fixTokenFallback(input: SuggestFixInput): FixSuggestion {
223238
}
224239

225240
if (input.issue === 'hardcoded-color') {
226-
const token = suggestTokenForProperty(property ?? 'color');
241+
const token = suggestTokenForProperty(property ?? 'color', tokenPrefix);
227242
// Extract the property and value
228243
const propMatch = original.match(/([a-z-]+)\s*:\s*([^;]+)/i);
229244
if (propMatch) {
@@ -250,10 +265,10 @@ function fixTokenFallback(input: SuggestFixInput): FixSuggestion {
250265
// ─── Theme compatibility fixes ───────────────────────────────────────────────
251266

252267
function fixThemeCompat(input: SuggestFixInput): FixSuggestion {
253-
const { original, property } = input;
268+
const { original, property, tokenPrefix } = input;
254269

255270
if (input.issue === 'hardcoded-color') {
256-
const token = suggestTokenForProperty(property ?? 'background');
271+
const token = suggestTokenForProperty(property ?? 'background', tokenPrefix);
257272
const propMatch = original.match(/([a-z-]+)\s*:\s*([^;]+)/i);
258273
if (propMatch) {
259274
const [, prop, value] = propMatch;
@@ -269,9 +284,11 @@ function fixThemeCompat(input: SuggestFixInput): FixSuggestion {
269284
}
270285

271286
if (input.issue === 'contrast-pair') {
287+
const bgToken = suggestTokenForProperty('background-color', tokenPrefix);
288+
const fgToken = suggestTokenForProperty('color', tokenPrefix);
272289
return {
273290
original,
274-
suggestion: `background: var(--sl-color-neutral-0); color: var(--sl-color-neutral-900);`,
291+
suggestion: `background: var(${bgToken}); color: var(${fgToken});`,
275292
explanation: `Light-on-light or dark-on-dark color pairs create contrast issues. Use semantic token pairs (surface + on-surface) that maintain readable contrast across themes.`,
276293
severity: 'warning',
277294
};

packages/core/src/tools/styling.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,7 @@ const SuggestFixArgsSchema = z.object({
148148
memberName: z.string().optional(),
149149
suggestedName: z.string().optional(),
150150
eventName: z.string().optional(),
151+
tokenPrefix: z.string().optional(),
151152
});
152153

153154
const ValidateComponentCodeArgsSchema = z.object({
@@ -622,6 +623,11 @@ export const STYLING_TOOL_DEFINITIONS = [
622623
type: 'string',
623624
description: 'Optional event name for event usage fixes.',
624625
},
626+
tokenPrefix: {
627+
type: 'string',
628+
description:
629+
'Optional token prefix from the component library (e.g. "--hx-", "--fast-", "--md-"). When provided, suggested replacement tokens use this prefix. Get this from diagnose_styling.',
630+
},
625631
},
626632
required: ['type', 'issue', 'original'],
627633
additionalProperties: false,

tests/handlers/styling-diagnostics.test.ts

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,19 @@ describe('buildAntiPatterns', () => {
215215
);
216216
expect(partWarning).toBeUndefined();
217217
});
218+
219+
it('includes font inheritance warning when slots exist', () => {
220+
const warnings = buildAntiPatterns(buttonMeta);
221+
const inheritWarning = warnings.find((w) => w.explanation.includes('inherit'));
222+
expect(inheritWarning).toBeDefined();
223+
expect(inheritWarning!.explanation).toContain('font');
224+
});
225+
226+
it('skips font inheritance warning when no slots', () => {
227+
const warnings = buildAntiPatterns(bareComponent);
228+
const inheritWarning = warnings.find((w) => w.explanation.includes('inherit'));
229+
expect(inheritWarning).toBeUndefined();
230+
});
218231
});
219232

220233
describe('buildCssSnippet', () => {
@@ -224,12 +237,55 @@ describe('buildCssSnippet', () => {
224237
expect(snippet).toContain('--my-button-bg');
225238
});
226239

240+
it('does NOT generate self-referential var() in token customization', () => {
241+
const snippet = buildCssSnippet(buttonMeta);
242+
// Should NOT contain --my-button-bg: var(--my-button-bg) — that's circular
243+
expect(snippet).not.toMatch(/--my-button-bg:\s*var\(--my-button-bg\)/);
244+
// Should contain a direct value assignment
245+
expect(snippet).toMatch(/--my-button-bg:\s*[^v]/);
246+
});
247+
248+
it('uses CEM default value when available', () => {
249+
const metaWithDefaults: ComponentMetadata = {
250+
...bareComponent,
251+
cssProperties: [
252+
{ name: '--my-card-bg', description: 'Background', default: '#ffffff' },
253+
{ name: '--my-card-radius', description: 'Radius', default: '4px' },
254+
],
255+
};
256+
const snippet = buildCssSnippet(metaWithDefaults);
257+
expect(snippet).toContain('#ffffff');
258+
expect(snippet).toContain('4px');
259+
});
260+
227261
it('includes part customization for components with CSS parts', () => {
228262
const snippet = buildCssSnippet(buttonMeta);
229263
expect(snippet).toContain('Part-based customization');
230264
expect(snippet).toContain('::part(base)');
231265
});
232266

267+
it('includes slot styling section when component has slots', () => {
268+
const snippet = buildCssSnippet(buttonMeta);
269+
expect(snippet).toContain('Slot styling');
270+
expect(snippet).toContain('light DOM');
271+
});
272+
273+
it('includes named slot selector example', () => {
274+
const snippet = buildCssSnippet(buttonMeta);
275+
expect(snippet).toContain('[slot="prefix"]');
276+
});
277+
278+
it('includes default slot child selector example', () => {
279+
const snippet = buildCssSnippet(buttonMeta);
280+
// Default slot: style children of the host
281+
expect(snippet).toContain('my-button >');
282+
});
283+
284+
it('skips slot section for components without slots', () => {
285+
const snippet = buildCssSnippet(bareComponent);
286+
expect(snippet).not.toContain('Slot styling');
287+
});
288+
233289
it('generates fallback message for bare components', () => {
234290
const snippet = buildCssSnippet(bareComponent);
235291
expect(snippet).toContain('no CSS customization');
@@ -248,9 +304,6 @@ describe('buildCssSnippet', () => {
248304
})),
249305
};
250306
const snippet = buildCssSnippet(manyProps);
251-
// Should have at most 5 properties listed (each appears twice: decl + var)
252-
const propMatches = snippet.match(/--prop-/g);
253-
expect(propMatches!.length).toBeLessThanOrEqual(10);
254307
// Should not contain --prop-5 through --prop-9
255308
expect(snippet).not.toContain('--prop-5');
256309
// Should have at most 3 parts listed

tests/handlers/suggest-fix.test.ts

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,70 @@ describe('suggestFix — layout', () => {
308308
});
309309
});
310310

311+
// ─── Library-agnostic token suggestions ─────────────────────────────────────
312+
313+
describe('suggestFix — library-agnostic tokens', () => {
314+
it('uses provided tokenPrefix instead of hardcoded --sl- tokens', () => {
315+
const result = suggestFix({
316+
type: 'token-fallback',
317+
issue: 'hardcoded-color',
318+
original: 'background-color: #3b82f6;',
319+
property: 'background-color',
320+
tokenPrefix: '--hx-',
321+
});
322+
expect(result.suggestion).toContain('--hx-');
323+
expect(result.suggestion).not.toContain('--sl-');
324+
});
325+
326+
it('uses generic placeholder when no tokenPrefix provided', () => {
327+
const result = suggestFix({
328+
type: 'token-fallback',
329+
issue: 'hardcoded-color',
330+
original: 'color: #333;',
331+
property: 'color',
332+
});
333+
// Should NOT contain library-specific prefix
334+
expect(result.suggestion).not.toContain('--sl-');
335+
expect(result.suggestion).toContain('var(');
336+
});
337+
338+
it('uses tokenPrefix in theme-compat hardcoded-color fix', () => {
339+
const result = suggestFix({
340+
type: 'theme-compat',
341+
issue: 'hardcoded-color',
342+
original: 'background: white;',
343+
property: 'background',
344+
tokenPrefix: '--fast-',
345+
});
346+
expect(result.suggestion).toContain('--fast-');
347+
expect(result.suggestion).not.toContain('--sl-');
348+
});
349+
350+
it('uses tokenPrefix in theme-compat contrast-pair fix', () => {
351+
const result = suggestFix({
352+
type: 'theme-compat',
353+
issue: 'contrast-pair',
354+
original: 'background: #f0f0f0; color: #e0e0e0;',
355+
tokenPrefix: '--md-',
356+
});
357+
expect(result.suggestion).toContain('--md-');
358+
expect(result.suggestion).not.toContain('--sl-');
359+
});
360+
361+
it('generates property-appropriate token names from prefix', () => {
362+
const result = suggestFix({
363+
type: 'token-fallback',
364+
issue: 'hardcoded-color',
365+
original: 'border-radius: 8px;',
366+
property: 'border-radius',
367+
tokenPrefix: '--hx-',
368+
});
369+
expect(result.suggestion).toContain('--hx-');
370+
// Should generate a radius-related token name
371+
expect(result.suggestion).toMatch(/--hx-.*radius/i);
372+
});
373+
});
374+
311375
// ─── Result structure ───────────────────────────────────────────────────────
312376

313377
describe('suggestFix — result structure', () => {

0 commit comments

Comments
 (0)