mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
fix: bound CSS seeding to real matches and ownership before supersession removal
Addresses two cursor findings on extractMatchingSourceCss plus an adjacent hazard in the same removal machinery: - Class matching is token-bounded, never substring: .btn no longer seeds .btn-primary and .stage no longer seeds .stages. A falsely seeded selector was an accept-time deletion of a hand-written rule, since any seeded selector the variant does not re-declare is removed as superseded. - Tag rules that style the pick (h1, a, p) now seed the preview stub, so unclassed selections start from the real cascade. They are excluded from the supersedable set: tag rules style shared elements across the route and must never be removal candidates. - Supersession removal is now bounded by ownership: a seeded class selector whose class is still used by markup OUTSIDE the replaced region survives the accept, because removing it would strip styling from markup the accept never touched. Tests cover substring non-matches, tag seeding with a tag-free supersedable set, and a shared-class accept where .card is used both inside the pick and elsewhere. AI-assisted (Claude Code). Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Code
parent
a83d767cf9
commit
20213a6817
@@ -207,16 +207,18 @@ export function scaffoldSvelteComponentSession({
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
|
||||
const contract = analysis.contract;
|
||||
const seededCss = extractMatchingSourceCss(
|
||||
const seeded = extractMatchingSourceCss(
|
||||
safeReadSource(path.resolve(cwd, sourceFile)),
|
||||
originalMarkup,
|
||||
);
|
||||
const seededCss = seeded.css;
|
||||
// The preview compiles in isolation, so NONE of these source rules applied
|
||||
// to what the user approved. Accept enforces that preview truth: any of
|
||||
// them the variant does not re-declare is superseded and removed, instead
|
||||
// of re-attaching to the accepted markup through kept class names (the
|
||||
// ".decisions grid grabs the new board" failure).
|
||||
const seededSelectors = [...collectAllSelectors(seededCss)];
|
||||
// ".decisions grid grabs the new board" failure). Only the CLASS-matched
|
||||
// selectors are candidates; tag rules style shared route elements.
|
||||
const seededSelectors = [...seeded.supersedable];
|
||||
|
||||
const manifest = {
|
||||
id,
|
||||
@@ -266,15 +268,26 @@ function safeReadSource(filePath) {
|
||||
try { return fs.readFileSync(filePath, 'utf-8'); } catch { return ''; }
|
||||
}
|
||||
|
||||
function escapeSelectorToken(token) {
|
||||
return String(token).replace(/[.*+?^${}()|[\]\\]/g, '\\$&');
|
||||
}
|
||||
|
||||
/**
|
||||
* Seed variant stubs with the source component's rules that already style the
|
||||
* selected markup, so variants start from the real cascade (a detached
|
||||
* preview inherits none of the route's compile-scoped CSS) instead of
|
||||
* reimplementing it blind.
|
||||
*
|
||||
* Returns { css, supersedable }. `css` is every matching rule (class OR tag
|
||||
* matched). `supersedable` holds only the CLASS-matched selectors: those are
|
||||
* the accept-time removal candidates. Tag selectors (h1, a, p) style shared
|
||||
* elements across the whole route, so they seed the preview but are never
|
||||
* candidates for removal.
|
||||
*/
|
||||
export function extractMatchingSourceCss(routeSource, originalMarkup) {
|
||||
const empty = { css: '', supersedable: new Set() };
|
||||
const styleMatch = String(routeSource || '').match(/<style\b[^>]*>([\s\S]*?)<\/style\s*>/i);
|
||||
if (!styleMatch) return '';
|
||||
if (!styleMatch) return empty;
|
||||
const classNames = new Set();
|
||||
const classRe = /class\s*=\s*(["'])(.*?)\1/g;
|
||||
let m;
|
||||
@@ -284,17 +297,35 @@ export function extractMatchingSourceCss(routeSource, originalMarkup) {
|
||||
const tagRe = /<([a-z][a-z0-9-]*)/gi;
|
||||
const tags = new Set();
|
||||
while ((m = tagRe.exec(originalMarkup))) tags.add(m[1].toLowerCase());
|
||||
if (classNames.size === 0 && tags.size === 0) return '';
|
||||
if (classNames.size === 0 && tags.size === 0) return empty;
|
||||
|
||||
const selectorMatches = (prelude) => splitSelectorList(prelude).some((selector) => {
|
||||
for (const cls of classNames) if (selector.includes(`.${cls}`)) return true;
|
||||
return false;
|
||||
});
|
||||
// Token-boundary matching, never substring: `.btn` must not match
|
||||
// `.btn-primary`, and `.stage` must not match `.stages`. A substring hit
|
||||
// seeds a rule that never styled the pick, and a falsely seeded selector
|
||||
// becomes an accept-time DELETION of a hand-written rule.
|
||||
const classRes = [...classNames].map((cls) => new RegExp('\\.' + escapeSelectorToken(cls) + '(?![A-Za-z0-9_-])'));
|
||||
const tagRes = [...tags].map((tag) => new RegExp('(^|[\\s>+~,(])' + escapeSelectorToken(tag) + '(?![A-Za-z0-9_-])', 'i'));
|
||||
const classMatches = (selector) => classRes.some((re) => re.test(selector));
|
||||
const tagMatches = (selector) => tagRes.some((re) => re.test(selector));
|
||||
|
||||
const supersedable = new Set();
|
||||
const ruleMatches = (prelude) => {
|
||||
let matched = false;
|
||||
for (const selector of splitSelectorList(prelude)) {
|
||||
if (classMatches(selector)) {
|
||||
matched = true;
|
||||
supersedable.add(normalizeSelector(selector));
|
||||
} else if (tagMatches(selector)) {
|
||||
matched = true;
|
||||
}
|
||||
}
|
||||
return matched;
|
||||
};
|
||||
|
||||
const pick = (nodes) => {
|
||||
const kept = [];
|
||||
for (const node of nodes) {
|
||||
if (node.type === 'rule' && selectorMatches(node.prelude)) kept.push(node);
|
||||
if (node.type === 'rule' && ruleMatches(node.prelude)) kept.push(node);
|
||||
else if (node.type === 'at' && node.children) {
|
||||
const children = pick(node.children);
|
||||
if (children.length) kept.push({ ...node, children });
|
||||
@@ -302,7 +333,7 @@ export function extractMatchingSourceCss(routeSource, originalMarkup) {
|
||||
}
|
||||
return kept;
|
||||
};
|
||||
return serializeNodes(pick(parseStylesheet(styleMatch[1])));
|
||||
return { css: serializeNodes(pick(parseStylesheet(styleMatch[1]))), supersedable };
|
||||
}
|
||||
|
||||
function buildVariantStubV2(variantNum, markupWithProps, contract, seededCss) {
|
||||
@@ -713,10 +744,37 @@ export function inlineSvelteComponentAccept(manifest, variantNum, paramValues =
|
||||
// them. Any seeded selector the variant did not re-declare is superseded;
|
||||
// left in place it re-attaches through kept class names (the accepted root
|
||||
// keeps its original classes) and re-layouts markup it no longer owns.
|
||||
//
|
||||
// Removal is bounded by ownership: a selector whose classes are still used
|
||||
// by route markup OUTSIDE the replaced region does not belong to the pick
|
||||
// alone, and removing it would strip styling from markup this accept never
|
||||
// touched. Keeping it risks a visible re-attachment quirk on the accepted
|
||||
// region; deleting it breaks the rest of the route. Keep it.
|
||||
const outsideMarkup = [...sourceLines.slice(0, start), ...sourceLines.slice(end + 1)]
|
||||
.join('\n')
|
||||
.replace(/<style\b[^>]*>[\s\S]*?<\/style\s*>/gi, '');
|
||||
const outsideClasses = new Set();
|
||||
{
|
||||
const attrRe = /class\s*=\s*(["'])(.*?)\1/g;
|
||||
let cm;
|
||||
while ((cm = attrRe.exec(outsideMarkup))) {
|
||||
for (const cls of cm[2].split(/\s+/)) if (cls && !cls.includes('{')) outsideClasses.add(cls);
|
||||
}
|
||||
const directiveRe = /class:([A-Za-z0-9_-]+)/g;
|
||||
while ((cm = directiveRe.exec(outsideMarkup))) outsideClasses.add(cm[1]);
|
||||
}
|
||||
const usedOutsideReplacedRegion = (selector) => {
|
||||
const classTokenRe = /\.([A-Za-z0-9_-]+)/g;
|
||||
let tm;
|
||||
while ((tm = classTokenRe.exec(selector))) {
|
||||
if (outsideClasses.has(tm[1])) return true;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
const incomingSelectors = collectAllSelectors(bakedCss);
|
||||
const superseded = (manifest.seededSelectors || [])
|
||||
.map((selector) => normalizeSelector(selector))
|
||||
.filter((selector) => selector && !incomingSelectors.has(selector));
|
||||
.filter((selector) => selector && !incomingSelectors.has(selector) && !usedOutsideReplacedRegion(selector));
|
||||
if (superseded.length > 0) {
|
||||
const scrubbed = removeSelectorsFromSvelteSource(finalText, new Set(superseded));
|
||||
finalText = scrubbed.text;
|
||||
|
||||
@@ -226,11 +226,43 @@ describe('svelte component scaffold + accept pipeline', () => {
|
||||
});
|
||||
|
||||
it('extractMatchingSourceCss picks only rules that style the selection', () => {
|
||||
const css = extractMatchingSourceCss(ROUTE_SOURCE, '<ol class="pit-board"><li class="stage">x</li></ol>');
|
||||
const { css } = extractMatchingSourceCss(ROUTE_SOURCE, '<ol class="pit-board"><li class="stage">x</li></ol>');
|
||||
assert.match(css, /\.pit-board/);
|
||||
assert.match(css, /\.stage/);
|
||||
assert.doesNotMatch(css, /\.footer/);
|
||||
});
|
||||
|
||||
it('class matching is token-bounded, never substring', () => {
|
||||
// The field hazard: a falsely seeded selector becomes an accept-time
|
||||
// DELETION of a hand-written rule the pick never used.
|
||||
const route = `<style>
|
||||
.btn { color: red; }
|
||||
.btn-primary { color: blue; }
|
||||
.stage { padding: 4px; }
|
||||
.stages { display: grid; }
|
||||
</style>`;
|
||||
const { css, supersedable } = extractMatchingSourceCss(route, '<button class="btn"><span class="stage">x</span></button>');
|
||||
assert.match(css, /\.btn \{/);
|
||||
assert.match(css, /\.stage \{/);
|
||||
assert.doesNotMatch(css, /btn-primary/, '.btn must not seed .btn-primary');
|
||||
assert.doesNotMatch(css, /\.stages/, '.stage must not seed .stages');
|
||||
assert.deepEqual([...supersedable].sort(), ['.btn', '.stage']);
|
||||
});
|
||||
|
||||
it('tag rules seed the preview but are never supersedable', () => {
|
||||
const route = `<style>
|
||||
h1 { font-size: 3rem; }
|
||||
h1.hero { letter-spacing: -0.02em; }
|
||||
p { line-height: 1.6; }
|
||||
.sidebar { width: 20rem; }
|
||||
</style>`;
|
||||
const { css, supersedable } = extractMatchingSourceCss(route, '<h1 class="hero">Title</h1>');
|
||||
assert.match(css, /h1 \{ font-size/, 'bare tag rules that style the pick are seeded');
|
||||
assert.match(css, /h1\.hero/, 'class rules still seed');
|
||||
assert.doesNotMatch(css, /^p \{/m, 'unrelated tags are not seeded');
|
||||
assert.doesNotMatch(css, /\.sidebar/);
|
||||
assert.deepEqual([...supersedable], ['h1.hero'], 'only class-matched selectors may be removed on accept');
|
||||
});
|
||||
});
|
||||
|
||||
describe('review regressions: preview-truth supersession (the Pitch mangle)', () => {
|
||||
@@ -421,3 +453,88 @@ describe('review regressions: publish-time compile gate', () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('review regressions: shared-class supersession guard', () => {
|
||||
const SHARED_SOURCE = `<script>
|
||||
let items = [{ name: 'a' }, { name: 'b' }];
|
||||
</script>
|
||||
|
||||
<section class="page">
|
||||
<div class="card intro">Intro copy stays here.</div>
|
||||
<ul class="list">
|
||||
{#each items as item}
|
||||
<li class="card">{item.name}</li>
|
||||
{/each}
|
||||
</ul>
|
||||
</section>
|
||||
|
||||
<style>
|
||||
.page { padding: 24px; }
|
||||
.card { border: 1px solid #999; border-radius: 8px; }
|
||||
.list { display: grid; gap: 8px; }
|
||||
</style>
|
||||
`;
|
||||
|
||||
it('keeps a superseded selector whose class is still used outside the replaced region', () => {
|
||||
const tmp4 = realpathSync(mkdtempSync(join(tmpdir(), 'impeccable-shared-class-')));
|
||||
try {
|
||||
mkdirSync(join(tmp4, 'node_modules'), { recursive: true });
|
||||
try {
|
||||
symlinkSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp4, 'node_modules', 'svelte'), 'dir');
|
||||
} catch {
|
||||
cpSync(join(REPO_NODE_MODULES, 'svelte'), join(tmp4, 'node_modules', 'svelte'), { recursive: true });
|
||||
}
|
||||
write(tmp4, 'package.json', JSON.stringify({ name: 'app' }));
|
||||
write(tmp4, 'src/lib/Shared.svelte', SHARED_SOURCE);
|
||||
|
||||
// Pick the <ul class="list"> block. Its items use .card, and so does
|
||||
// the intro div OUTSIDE the pick.
|
||||
const lines = SHARED_SOURCE.split('\n');
|
||||
const startLine = lines.findIndex((l) => l.includes('class="list"')) + 1;
|
||||
const endLine = lines.findIndex((l) => l.trim() === '</ul>') + 1;
|
||||
const originalLines = lines.slice(startLine - 1, endLine);
|
||||
|
||||
const session = scaffoldSvelteComponentSession({
|
||||
id: 'shared01',
|
||||
count: 1,
|
||||
sourceFile: 'src/lib/Shared.svelte',
|
||||
sourceStartLine: startLine,
|
||||
sourceEndLine: endLine,
|
||||
originalLines,
|
||||
cwd: tmp4,
|
||||
});
|
||||
assert.equal(session.fallback, undefined, session.reason);
|
||||
assert.equal(session.manifest.seededSelectors.includes('.card'), true, 'the pick uses .card, so it seeds');
|
||||
|
||||
// The variant re-declares .list but NOT .card.
|
||||
write(tmp4, join(session.componentDir, 'v1.svelte'), `<script>
|
||||
let { items = [] } = $props();
|
||||
</script>
|
||||
|
||||
<ul class="list">
|
||||
{#each items as item}
|
||||
<li class="card">{item.name}</li>
|
||||
{/each}
|
||||
</ul>
|
||||
|
||||
<style>
|
||||
.list { display: flex; flex-direction: column; gap: 12px; }
|
||||
</style>
|
||||
`);
|
||||
const manifest = findSvelteComponentManifest('shared01', tmp4);
|
||||
const result = inlineSvelteComponentAccept(manifest, 1, null, tmp4);
|
||||
assert.equal(result.handled, true, result.error);
|
||||
const out = readFileSync(join(tmp4, 'src/lib/Shared.svelte'), 'utf-8');
|
||||
|
||||
// .card is shared with the intro div outside the replaced region:
|
||||
// removing it would strip styling from markup this accept never
|
||||
// touched, so it must survive despite not being re-declared.
|
||||
assert.match(out, /\.card \{ border: 1px solid #999; border-radius: 8px; \}/);
|
||||
assert.equal(result.css.superseded.includes('.card'), false);
|
||||
// The re-declared .list took the variant's shape.
|
||||
assert.match(out, /\.list \{ display: flex/);
|
||||
} finally {
|
||||
rmSync(tmp4, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user