mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
Fix: inherit the monorepo root's DESIGN.md in detect design-system rules (#570)
findDesignRoot stopped at the first package.json boundary, so every design-system rule silently abstained for files inside monorepo workspaces. The walk now continues past a workspace boundary to the monorepo root that owns it, recognized the same way context.mjs does (declared workspace globs, or a marker file beside apps/ or packages/ children). A nested repo with its own .git, a workspace-owned DESIGN.md, and non-monorepo projects keep their existing behavior. Written with AI assistance (Cursor); reviewed by maintainer. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Abdul Wahab
Cursor
@@ -13,6 +13,11 @@ const FALLBACK_DIRS = ['.agents/context', 'docs'];
|
||||
// CLI can't import (separate tree). `.git` and `package.json` are the common
|
||||
// boundaries; `.impeccable` is our own project marker.
|
||||
const PROJECT_ROOT_MARKERS = ['.git', 'package.json', '.impeccable'];
|
||||
// Monorepo-root recognition, mirroring context.mjs's isMonorepoRoot: declared
|
||||
// workspace globs (package.json `workspaces`, pnpm-workspace.yaml `packages:`)
|
||||
// or a marker file beside apps/ or packages/ children.
|
||||
const MONOREPO_MARKER_FILES = ['pnpm-workspace.yaml', 'turbo.json', 'nx.json', 'lerna.json'];
|
||||
const MONOREPO_FALLBACK_PROJECT_DIRS = ['apps', 'packages'];
|
||||
const COLOR_CHANNEL_TOLERANCE = 6;
|
||||
// Shadow blacks at different alphas are different tokens (0.28 vs 0.55 is the
|
||||
// difference between a documented shadow and drift), so shadow matching cannot
|
||||
@@ -581,24 +586,77 @@ function designSystemStartDir(targetPath, cwd = process.cwd()) {
|
||||
// - A directory carrying a DESIGN.md (directly or in a fallback dir) IS the
|
||||
// design root — that's where the rules live.
|
||||
// - A directory carrying a project marker (.git / package.json / .impeccable)
|
||||
// but no DESIGN.md is a project BOUNDARY: the walk stops with no design
|
||||
// system, so a sibling project never inherits a parent's or cwd's rules.
|
||||
// but no DESIGN.md is a project BOUNDARY. When that boundary is a monorepo
|
||||
// workspace (owned by a workspace-declaring root above it, recognized the
|
||||
// same way context.mjs does), the workspace inherits the monorepo root's
|
||||
// DESIGN.md; a nested separate repository (.git with no workspace
|
||||
// declaration) still inherits nothing (issue #570).
|
||||
// - Reaching the home directory / filesystem root with neither means no
|
||||
// design system at all — never process.cwd()'s.
|
||||
//
|
||||
// Returns { dir, hasDesign } for the stopping directory, or null when the walk
|
||||
// runs out. This is the fix for cross-project contamination.
|
||||
function readWorkspacePatterns(dir) {
|
||||
const workspaces = safeReadJson(path.join(dir, 'package.json'))?.workspaces;
|
||||
const patterns = Array.isArray(workspaces) ? [...workspaces]
|
||||
: Array.isArray(workspaces?.packages) ? [...workspaces.packages] : [];
|
||||
try {
|
||||
let inPackages = false;
|
||||
for (const line of fs.readFileSync(path.join(dir, 'pnpm-workspace.yaml'), 'utf-8').split(/\r?\n/)) {
|
||||
const trimmed = line.trim();
|
||||
if (!trimmed || trimmed.startsWith('#')) continue;
|
||||
const flow = trimmed.match(/^packages:\s*\[(.*)\]\s*$/);
|
||||
if (flow) {
|
||||
patterns.push(...flow[1].split(',').map(entry => entry.trim().replace(/^['"]|['"]$/g, '')).filter(Boolean));
|
||||
break;
|
||||
}
|
||||
if (/^packages:\s*$/.test(trimmed)) { inPackages = true; continue; }
|
||||
if (!inPackages) continue;
|
||||
const item = trimmed.match(/^-\s*(.+)$/);
|
||||
if (item) patterns.push(item[1].trim().replace(/^['"]|['"]$/g, ''));
|
||||
else if (/^[A-Za-z0-9_-]+:\s*/.test(trimmed)) break;
|
||||
}
|
||||
} catch { /* no pnpm-workspace.yaml */ }
|
||||
return patterns;
|
||||
}
|
||||
|
||||
function isMonorepoRoot(dir) {
|
||||
if (readWorkspacePatterns(dir).some(pattern => !String(pattern).trim().startsWith('!'))) return true;
|
||||
if (!MONOREPO_MARKER_FILES.some(file => fs.existsSync(path.join(dir, file)))) return false;
|
||||
return MONOREPO_FALLBACK_PROJECT_DIRS.some(name => {
|
||||
try {
|
||||
return fs.readdirSync(path.join(dir, name), { withFileTypes: true }).some(entry => entry.isDirectory());
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
export function findDesignRoot(startDir) {
|
||||
let dir = path.resolve(startDir);
|
||||
const homeDir = path.resolve(os.homedir());
|
||||
let boundary = null;
|
||||
while (true) {
|
||||
if (resolveDesignMdPath(dir)) return { dir, hasDesign: true };
|
||||
if (PROJECT_ROOT_MARKERS.some((marker) => fs.existsSync(path.join(dir, marker)))) {
|
||||
return { dir, hasDesign: false };
|
||||
if (!boundary && resolveDesignMdPath(dir)) return { dir, hasDesign: true };
|
||||
if (boundary) {
|
||||
// Past the boundary the walk only looks for the monorepo root that owns
|
||||
// the workspace. Monorepo-root before .git, same order as context.mjs:
|
||||
// a workspace root carrying its own .git is still recognized, while a
|
||||
// .git that declares no workspaces is a separate repository and stops
|
||||
// the walk with nothing inherited. The home directory is never an
|
||||
// owning root, same as context.mjs's findMonorepoRoot, which stops at
|
||||
// homeDir before its monorepo check.
|
||||
if (dir !== homeDir && isMonorepoRoot(dir)) return { dir, hasDesign: !!resolveDesignMdPath(dir) };
|
||||
if (fs.existsSync(path.join(dir, '.git'))) return boundary;
|
||||
} else if (PROJECT_ROOT_MARKERS.some((marker) => fs.existsSync(path.join(dir, marker)))) {
|
||||
boundary = { dir, hasDesign: false };
|
||||
// A boundary that is itself a monorepo root, or a separate repository
|
||||
// with its own .git, inherits nothing from above.
|
||||
if (isMonorepoRoot(dir) || fs.existsSync(path.join(dir, '.git'))) return boundary;
|
||||
}
|
||||
if (dir === homeDir) return null;
|
||||
if (dir === homeDir) return boundary;
|
||||
const parent = path.dirname(dir);
|
||||
if (parent === dir) return null;
|
||||
if (parent === dir) return boundary;
|
||||
dir = parent;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -107,6 +107,7 @@ export const SUITES = {
|
||||
'tests/detect-antipatterns-fixtures.test.mjs',
|
||||
'tests/detect-antipatterns-browser.test.mjs',
|
||||
'tests/detect-cli-design-contamination.test.mjs',
|
||||
'tests/detect-cli-design-monorepo.test.mjs',
|
||||
'tests/detect-cli-stdin-dispatch.test.mjs',
|
||||
],
|
||||
},
|
||||
|
||||
@@ -0,0 +1,181 @@
|
||||
/**
|
||||
* Regression for issue #570: design-system rules must reach a monorepo workspace
|
||||
* by inheriting the repo root's DESIGN.md.
|
||||
*
|
||||
* Run with: node --test tests/detect-cli-design-monorepo.test.mjs
|
||||
*/
|
||||
|
||||
import { describe, it, after } from 'node:test';
|
||||
import assert from 'node:assert/strict';
|
||||
import fs from 'node:fs';
|
||||
import os from 'node:os';
|
||||
import path from 'node:path';
|
||||
import { spawnSync } from 'node:child_process';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const __dirname = path.dirname(fileURLToPath(import.meta.url));
|
||||
const CLI = path.resolve(__dirname, '../cli/bin/cli.js');
|
||||
|
||||
const PAGE_HTML =
|
||||
'<!doctype html><html><head><style>.card { font-family: Verdana, sans-serif; }</style></head>' +
|
||||
'<body><div class="card">Hi</div></body></html>';
|
||||
|
||||
const DESIGN_MD = `---
|
||||
typography:
|
||||
body:
|
||||
fontFamily: "Palatino, Georgia, serif"
|
||||
---
|
||||
# Project A Design System
|
||||
`;
|
||||
|
||||
const tempRoots = [];
|
||||
|
||||
function runDetect(cwd, targets, env = {}) {
|
||||
const result = spawnSync(process.execPath, [CLI, 'detect', '--json', ...targets], {
|
||||
cwd,
|
||||
encoding: 'utf-8',
|
||||
env: { ...process.env, ...env },
|
||||
});
|
||||
let findings = [];
|
||||
try {
|
||||
findings = JSON.parse(result.stdout || '[]');
|
||||
} catch {
|
||||
throw new Error(`Non-JSON CLI output.\nstdout: ${result.stdout}\nstderr: ${result.stderr}`);
|
||||
}
|
||||
return findings;
|
||||
}
|
||||
|
||||
function fontFindingsFor(findings, file) {
|
||||
return findings.filter(
|
||||
(f) => f.antipattern === 'design-system-font' && (!file || f.file === file),
|
||||
);
|
||||
}
|
||||
|
||||
function mkPnpmMonorepo({ workspaceDesign = null } = {}) {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-pnpm-'));
|
||||
tempRoots.push(dir);
|
||||
fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD);
|
||||
fs.writeFileSync(path.join(dir, 'pnpm-workspace.yaml'), "packages:\n - 'apps/*'\n");
|
||||
fs.mkdirSync(path.join(dir, 'apps/web'), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'apps/web/package.json'), '{"name":"web"}');
|
||||
const page = path.join(dir, 'apps/web/page.html');
|
||||
fs.writeFileSync(page, PAGE_HTML);
|
||||
if (workspaceDesign) {
|
||||
fs.writeFileSync(path.join(dir, 'apps/web/DESIGN.md'), workspaceDesign);
|
||||
}
|
||||
return { dir, page, webDir: path.join(dir, 'apps/web') };
|
||||
}
|
||||
|
||||
after(() => {
|
||||
for (const dir of tempRoots) {
|
||||
try { fs.rmSync(dir, { recursive: true, force: true }); } catch { /* best effort */ }
|
||||
}
|
||||
});
|
||||
|
||||
describe('detect CLI monorepo DESIGN.md inheritance', () => {
|
||||
it('pnpm workspace root: workspace page inherits root DESIGN.md', () => {
|
||||
const { dir, page } = mkPnpmMonorepo();
|
||||
const findings = runDetect(dir, [page]);
|
||||
assert.ok(
|
||||
fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'),
|
||||
'Verdana must be flagged via inherited root DESIGN.md',
|
||||
);
|
||||
});
|
||||
|
||||
it('npm/yarn workspaces root: workspace page inherits root DESIGN.md', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-npm-'));
|
||||
tempRoots.push(dir);
|
||||
fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD);
|
||||
fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"mono","workspaces":["packages/*"]}');
|
||||
fs.mkdirSync(path.join(dir, 'packages/ui'), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'packages/ui/package.json'), '{"name":"ui"}');
|
||||
const page = path.join(dir, 'packages/ui/page.html');
|
||||
fs.writeFileSync(page, PAGE_HTML);
|
||||
|
||||
const findings = runDetect(dir, [page]);
|
||||
assert.ok(
|
||||
fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'),
|
||||
'Verdana must be flagged via inherited root DESIGN.md',
|
||||
);
|
||||
});
|
||||
|
||||
it('turbo marker root: workspace page inherits root DESIGN.md', () => {
|
||||
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-turbo-'));
|
||||
tempRoots.push(dir);
|
||||
fs.writeFileSync(path.join(dir, 'DESIGN.md'), DESIGN_MD);
|
||||
fs.writeFileSync(path.join(dir, 'package.json'), '{"name":"mono"}');
|
||||
fs.writeFileSync(path.join(dir, 'turbo.json'), '{}');
|
||||
fs.mkdirSync(path.join(dir, 'apps/web'), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'apps/web/package.json'), '{"name":"web"}');
|
||||
const page = path.join(dir, 'apps/web/page.html');
|
||||
fs.writeFileSync(page, PAGE_HTML);
|
||||
|
||||
const findings = runDetect(dir, [page]);
|
||||
assert.ok(
|
||||
fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'),
|
||||
'Verdana must be flagged via inherited root DESIGN.md',
|
||||
);
|
||||
});
|
||||
|
||||
it('directory target: scan apps/web dir inherits root DESIGN.md', () => {
|
||||
const { dir, page, webDir } = mkPnpmMonorepo();
|
||||
const findings = runDetect(dir, [webDir]);
|
||||
assert.ok(
|
||||
fontFindingsFor(findings, page).some((f) => f.ignoreValue === 'verdana'),
|
||||
'Verdana must be flagged when scanning the workspace directory',
|
||||
);
|
||||
});
|
||||
|
||||
it('workspace-owned DESIGN.md wins over monorepo root', () => {
|
||||
const workspaceDesign = `---
|
||||
typography:
|
||||
body:
|
||||
fontFamily: "Verdana, sans-serif"
|
||||
---
|
||||
# Workspace Design System
|
||||
`;
|
||||
const { dir, page } = mkPnpmMonorepo({ workspaceDesign });
|
||||
const findings = runDetect(dir, [page]);
|
||||
assert.equal(
|
||||
fontFindingsFor(findings, page).length,
|
||||
0,
|
||||
'workspace DESIGN.md allowing Verdana must suppress inherited root rules',
|
||||
);
|
||||
});
|
||||
|
||||
it('nested separate repo inherits nothing from monorepo root', () => {
|
||||
const { dir } = mkPnpmMonorepo();
|
||||
fs.mkdirSync(path.join(dir, 'vendor/other/.git'), { recursive: true });
|
||||
fs.writeFileSync(path.join(dir, 'vendor/other/package.json'), '{"name":"other"}');
|
||||
const page = path.join(dir, 'vendor/other/page.html');
|
||||
fs.writeFileSync(page, PAGE_HTML);
|
||||
|
||||
const findings = runDetect(dir, [page]);
|
||||
assert.equal(
|
||||
fontFindingsFor(findings, page).length,
|
||||
0,
|
||||
'nested repo with no workspaces must not inherit monorepo root DESIGN.md',
|
||||
);
|
||||
});
|
||||
|
||||
it('home directory is never an owning monorepo root', () => {
|
||||
// context.mjs's findMonorepoRoot stops at homeDir before its monorepo
|
||||
// check; the engine walk must match, or a workspace-declaring $HOME
|
||||
// leaks its DESIGN.md into every git-less project beneath it.
|
||||
const home = fs.mkdtempSync(path.join(os.tmpdir(), 'impeccable-detect-mono-home-'));
|
||||
tempRoots.push(home);
|
||||
fs.writeFileSync(path.join(home, 'DESIGN.md'), DESIGN_MD);
|
||||
fs.writeFileSync(path.join(home, 'pnpm-workspace.yaml'), "packages:\n - 'apps/*'\n");
|
||||
fs.mkdirSync(path.join(home, 'project'), { recursive: true });
|
||||
fs.writeFileSync(path.join(home, 'project/package.json'), '{"name":"p"}');
|
||||
const page = path.join(home, 'project/page.html');
|
||||
fs.writeFileSync(page, PAGE_HTML);
|
||||
|
||||
const findings = runDetect(home, [page], { HOME: home, USERPROFILE: home });
|
||||
assert.equal(
|
||||
fontFindingsFor(findings, page).length,
|
||||
0,
|
||||
'a project under a workspace-declaring $HOME must not inherit its DESIGN.md',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user