diff --git a/.changeset/prevent-css-name-overwrites.md b/.changeset/prevent-css-name-overwrites.md new file mode 100644 index 000000000..e6879ee61 --- /dev/null +++ b/.changeset/prevent-css-name-overwrites.md @@ -0,0 +1,13 @@ +--- +'@clickhouse/click-ui': patch +--- + +Add validation to prevent CSS file name collisions at build time. + +**Why this matters:** + +When a component uses CSS Modules (e.g., `Button.module.css`), the build process generates a processed CSS file (e.g., `.css-modules-temp/components/Button.css`). If a regular CSS file with the same name exists in the source (e.g., `src/components/Button.css`), both would attempt to write to `dist/components/Button.css`, causing ambiguity about which file should take precedence. + +**What changed:** + +Added early validation (`preventCssNameOverwrites`) that throws a clear error if such a naming conflict is detected, rather than silently overwriting files. The check runs once before the build since it validates source file names, which are format-independent. diff --git a/.llm/CONVENTIONS.md b/.llm/CONVENTIONS.md index 8c24f9903..50a3d81dd 100644 --- a/.llm/CONVENTIONS.md +++ b/.llm/CONVENTIONS.md @@ -49,6 +49,30 @@ When using CSS Modules (migration in progress from styled-components): - Use CSS custom properties from theme tokens: `var(--click-button-basic-color-primary-background-default)` - Always include `:focus-visible` styles for keyboard accessibility, never use `outline: none` without replacement +### CSS File Naming (Prevent Overwrites) + +⚠️ **CRITICAL**: Never have both `{name}.module.css` and `{name}.css` in the same directory. + +**Context**: Click UI uses CSS colocation, each component ships with its CSS alongside it. CSS Modules (`.module.css`) are preprocessed during build into standard CSS files and both are copied to `dist/` maintaining the same directory structure. This enables bundlers to discover and include CSS automatically when importing components. + +**Why**: During build, `.module.css` files are processed and written as `.css` to the output. If a regular `.css` file exists with the same name, both would attempt to write to the same destination path in `dist/`, causing ambiguity. + +**Wrong:** +``` +src/components/Button/ +├── Button.module.css ← Processed to dist/components/Button.css +└── Button.css ← Also writes to dist/components/Button.css +``` + +**Correct:** +``` +src/components/Button/ +├── Button.module.css ← CSS Modules (scoped) +└── Button.theme.css ← Different name, distinct output +``` + +The build will throw an error if this collision is detected, but it's better to prevent it at the source. + ### Accessibility (Mandatory) - Interactive elements need `role`, `aria-label`, `aria-describedby` @@ -122,3 +146,4 @@ src/components/ - `React.FC` or explicit `children` in props (use `React.ReactNode`) - Circular imports via barrel files - Untyped event handlers +- Having both `{name}.module.css` and `{name}.css` in same directory (causes dist overwrite collision) diff --git a/eslint.config.js b/eslint.config.js index aafe5a6fd..1cc530b4b 100644 --- a/eslint.config.js +++ b/eslint.config.js @@ -125,5 +125,14 @@ export default tseslint.config( '@typescript-eslint/no-unused-expressions': 'off', }, }, + { + files: ['plugins/**/*.ts'], + languageOptions: { + parserOptions: { + project: './tsconfig.node.json', + tsconfigRootDir: import.meta.dirname, + }, + }, + }, ...storybook.configs['flat/recommended'] ); diff --git a/package.json b/package.json index 2743ced4f..ff32c04d9 100644 --- a/package.json +++ b/package.json @@ -379,8 +379,8 @@ "generate:tokens": "node ./.scripts/js/generate-tokens.js && yarn format:fix src/theme/tokens/**/*.ts", "lint": "yarn lint:code && yarn lint:css", "lint:fix": "yarn lint:code:fix && yarn lint:css:fix", - "lint:code": "eslint src --report-unused-disable-directives", - "lint:code:fix": "eslint src --report-unused-disable-directives --fix", + "lint:code": "eslint src plugins --report-unused-disable-directives", + "lint:code:fix": "eslint src plugins --report-unused-disable-directives --fix", "lint:css": "stylelint \"src/**/*.css\"", "lint:css:fix": "stylelint \"src/**/*.css\" --fix", "prepare": "husky", diff --git a/plugins/css-colocate/css-preprocess.ts b/plugins/css-colocate/css-preprocess.ts index d84d19333..91f41653d 100644 --- a/plugins/css-colocate/css-preprocess.ts +++ b/plugins/css-colocate/css-preprocess.ts @@ -4,7 +4,7 @@ import postcss from 'postcss'; import postcssModules from 'postcss-modules'; import { getTempDir, findFiles, generateScopedName } from './utils'; -export async function preprocessCssModules(rootDir: string): Promise { +export const preprocessCssModules = async (rootDir: string): Promise => { const srcDir = path.join(rootDir, 'src'); const tempDir = getTempDir(rootDir); @@ -50,4 +50,4 @@ export async function preprocessCssModules(rootDir: string): Promise { console.log( `\n✅ Pre-processing complete: ${files.length} file(s), ${totalClasses} class(es)\n` ); -} +}; diff --git a/plugins/css-colocate/import-inject.ts b/plugins/css-colocate/import-inject.ts index 46b6164d0..2e0806422 100644 --- a/plugins/css-colocate/import-inject.ts +++ b/plugins/css-colocate/import-inject.ts @@ -52,7 +52,7 @@ const copyAndResolveCss = async ( jsOutputFile: string ): Promise => { const cssSourcePath = resolveCssPath(cssImportPath, sourceFile, srcDir); - if (!cssSourcePath || !(await fileExists(cssSourcePath))) return null; + if (!cssSourcePath || !(await fileExists(cssSourcePath))) {return null;} const cssRelativeToSrc = path.relative(srcDir, cssSourcePath); const cssOutputPath = path.join(distDir, cssRelativeToSrc); @@ -111,10 +111,10 @@ export const injectComponentCss = async ( const component = path.basename(dir); const cssFile = path.join(dir, `${component}.css`); - if (!(await fileExists(cssFile))) continue; + if (!(await fileExists(cssFile))) {continue;} const content = await fs.readFile(jsFile, 'utf-8'); - if (content.includes(`${component}.css`)) continue; + if (content.includes(`${component}.css`)) {continue;} const importStmt = createImportStatement(`./${component}.css`, format) + '\n'; const updated = insertAtTop(content, importStmt); @@ -133,7 +133,7 @@ export const injectRegularCssImports = async ( distDir: string, format: 'esm' | 'cjs' ): Promise => { - if (trackedImports.length === 0) return; + if (trackedImports.length === 0) {return;} const srcDir = path.join(rootDir, 'src'); @@ -144,7 +144,7 @@ export const injectRegularCssImports = async ( relativeToSrc.replace(/\.tsx?$/, `.${format === 'esm' ? 'js' : 'cjs'}`) ); - if (!(await fileExists(jsOutputFile))) continue; + if (!(await fileExists(jsOutputFile))) {continue;} let content = await fs.readFile(jsOutputFile, 'utf-8'); diff --git a/plugins/css-colocate/index.ts b/plugins/css-colocate/index.ts index 607fec190..58882e318 100644 --- a/plugins/css-colocate/index.ts +++ b/plugins/css-colocate/index.ts @@ -2,7 +2,7 @@ import type { Plugin, ResolvedConfig } from 'vite'; import { preprocessCssModules } from './css-preprocess'; import { resolveCssModule, loadCssModule } from './virtual-modules'; import { injectComponentCss, injectRegularCssImports } from './import-inject'; -import { copyCssFiles } from './utils'; +import { copyCssFiles, preventCssNameOverwrites } from './utils'; import path from 'path'; interface TrackedCssImport { @@ -27,29 +27,28 @@ export const cssColocatePlugin = (): Plugin => { name: 'vite-plugin-css-colocate', apply: 'build', - async buildStart() { + buildStart: async () => { // WARN: Reset between rebuilds // to prevent future build:watch transform to keep // appending, causing duplicate CSS imports // on each rebuild trackedImports.length = 0; + await preprocessCssModules(config.root); }, - configResolved(resolvedConfig) { + configResolved: resolvedConfig => { config = resolvedConfig; }, - async resolveId(id, importer) { - return resolveCssModule(id, importer, config.root); - }, + resolveId: async (id, importer) => resolveCssModule(id, importer, config.root), async load(id) { return loadCssModule(id, this, config.root); }, - transform(code, id) { - if (id.includes('node_modules') || !/\.[jt]sx?$/.test(id)) return null; + transform: (code, id) => { + if (id.includes('node_modules') || !/\.[jt]sx?$/.test(id)) {return null;} // Track regular CSS imports (not .module.css) const cssImports: string[] = []; @@ -69,22 +68,22 @@ export const cssColocatePlugin = (): Plugin => { return null; }, - async closeBundle() { + closeBundle: async () => { const formats = [ { format: 'esm' as const, ext: 'js' as const }, { format: 'cjs' as const, ext: 'cjs' as const }, ]; + // WARN: Prevents CSS file name collisions between processed .module.css files and regular .css files. E.g. throws an error if a .module.css file would produce the same output name as a .css file. This check is format-independent because it validates source files, not output. + await preventCssNameOverwrites(config.root); + for (const { format, ext } of formats) { const distDir = path.join(config.root, 'dist', format); - // Copy all CSS files (from temp and src) await copyCssFiles(config.root, distDir); - // Inject CSS imports into component files await injectComponentCss(distDir, format, ext); - // Inject CSS imports into files with tracked imports await injectRegularCssImports(trackedImports, config.root, distDir, format); } }, diff --git a/plugins/css-colocate/utils.ts b/plugins/css-colocate/utils.ts index 0988ed30b..ef4ca6099 100644 --- a/plugins/css-colocate/utils.ts +++ b/plugins/css-colocate/utils.ts @@ -32,12 +32,40 @@ export const createImportStatement = ( return format === 'esm' ? `import "${importPath}";` : `require("${importPath}");`; }; -export const copyCssFiles = async (rootDir: string, distDir: string): Promise => { +/** + * Prevents CSS file name collisions between processed .module.css files and regular .css files. + * Throws an error if a .module.css file would produce the same output name as a .css file. + * This check is format-independent because it validates source files, not output. + */ +export const preventCssNameOverwrites = async (rootDir: string): Promise => { const tempDir = getTempDir(rootDir); const srcDir = path.join(rootDir, 'src'); - // Track processed CSS files to detect naming collisions with regular CSS - const copiedFromTemp = new Set(); + const processedCssDestPaths = new Set(); + + const tempFiles = await findFiles(tempDir, '**/*.css'); + for (const file of tempFiles) { + const destPath = path.relative(tempDir, file); + processedCssDestPaths.add(destPath); + } + + const srcFiles = await findFiles(srcDir, '**/*.css'); + for (const file of srcFiles) { + if (file.endsWith('.module.css')) {continue;} + + const destPath = path.relative(srcDir, file); + + if (processedCssDestPaths.has(destPath)) { + throw new Error( + `👹 Oops! CSS naming collision detected: "${destPath}" exists as both a processed .module.css file and a regular .css file. Please rename one of them to avoid ambiguity.` + ); + } + } +}; + +export const copyCssFiles = async (rootDir: string, distDir: string): Promise => { + const tempDir = getTempDir(rootDir); + const srcDir = path.join(rootDir, 'src'); // Copy processed CSS from temp (generated from .module.css files) // These are processed through postcss-modules with hashed class names @@ -47,27 +75,15 @@ export const copyCssFiles = async (rootDir: string, distDir: string): Promise { - if (file.endsWith('.module.css')) return; + if (file.endsWith('.module.css')) {return;} const dest = path.join(distDir, path.relative(srcDir, file)); - - // Check for naming collision with processed CSS - if (copiedFromTemp.has(dest)) { - const relativePath = path.relative(distDir, dest); - throw new Error( - `CSS naming collision detected: "${relativePath}" exists as both a processed .module.css file and a regular .css file. ` + - `Please rename one of them to avoid ambiguity.` - ); - } - await fs.ensureDir(path.dirname(dest)); await fs.copy(file, dest, { overwrite: true }); }) diff --git a/plugins/css-colocate/virtual-modules.ts b/plugins/css-colocate/virtual-modules.ts index eebc24100..999efc89c 100644 --- a/plugins/css-colocate/virtual-modules.ts +++ b/plugins/css-colocate/virtual-modules.ts @@ -5,12 +5,14 @@ import { getTempDir } from './utils'; const VIRTUAL_PREFIX = 'virtual:css-module:'; -export async function resolveCssModule( +export const resolveCssModule = async ( id: string, importer: string | undefined, rootDir: string -): Promise { - if (!id.endsWith('.module.css') || !importer) return null; +): Promise => { + if (!id.endsWith('.module.css') || !importer) { + return null; + } const resolved = path.resolve(path.dirname(importer), id); const relative = path.relative(path.join(rootDir, 'src'), resolved); @@ -19,17 +21,21 @@ export async function resolveCssModule( relative.replace('.module.css', '.module.json') ); - if (!(await fs.pathExists(jsonPath))) return null; + if (!(await fs.pathExists(jsonPath))) { + return null; + } return VIRTUAL_PREFIX + relative; -} +}; -export async function loadCssModule( +export const loadCssModule = async ( id: string, ctx: PluginContext, rootDir: string -): Promise { - if (!id.startsWith(VIRTUAL_PREFIX)) return null; +): Promise => { + if (!id.startsWith(VIRTUAL_PREFIX)) { + return null; + } const relative = id.slice(VIRTUAL_PREFIX.length); const jsonPath = path.join( @@ -48,7 +54,8 @@ export async function loadCssModule( }) .join(',\n '); return `export default {\n ${exports}\n};`; - } catch (e: any) { - ctx.error(`Failed to load CSS module from ${jsonPath}: ${e.message}`); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + ctx.error(`Failed to load CSS module from ${jsonPath}: ${msg}`); } -} +};