-
Notifications
You must be signed in to change notification settings - Fork 40
chore(atomic): add script / CI job to validate light DOM styles in Lit components #6807
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
eb2ea7a
add static analysis script for detecting invalid :host selectors in L…
fbeaudoincoveo 8da833f
add validation script for light DOM host selectors
fbeaudoincoveo 99be7fc
add validation step for Light DOM styles in CI workflow
fbeaudoincoveo 848e409
[WIP] Address feedback on light DOM styles validation script (#6811)
Copilot 498239d
[WIP] Address feedback on validating light DOM styles in Lit componen…
Copilot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
319 changes: 319 additions & 0 deletions
319
packages/atomic/scripts/check-light-dom-host-selectors.mjs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,319 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| /** | ||
| * Static analysis script to detect invalid `:host` selectors in Light DOM components. | ||
| * | ||
| * Problem: When a Lit component uses LightDomMixin (no Shadow DOM), the `:host` CSS selector | ||
| * doesn't work as intended because there's no shadow boundary. This can cause styles to leak | ||
| * onto parent elements, leading to subtle production bugs. | ||
| * | ||
| * This script: | ||
| * 1. Finds all Lit components that use LightDomMixin or ItemSectionMixin | ||
| * 2. Checks if they have `static styles` with `:host` selector (inline or imported) | ||
| * 3. Recursively follows style imports to detect `:host` in imported CSS files | ||
| * 4. Reports any violations | ||
| * | ||
| * Usage: | ||
| * node scripts/check-light-dom-host-selectors.mjs | ||
| * | ||
| * Exit codes: | ||
| * 0 - No violations found | ||
| * 1 - Violations found or script error | ||
| */ | ||
|
|
||
| import {existsSync, readdirSync, readFileSync, statSync} from 'node:fs'; | ||
| import {dirname, extname, join, resolve} from 'node:path'; | ||
|
|
||
| const ATOMIC_SRC = new URL('../src', import.meta.url).pathname; | ||
|
|
||
| const STATIC_STYLES_PATTERN = /static\s+(?:(?:override\s+)?styles)\s*[=:]/; | ||
| const HOST_SELECTOR_PATTERN = /:host(?![a-z-])/; | ||
| const IMPORT_PATTERN = /import\s+(?:[\w{}\s,*]+\s+from\s+)?['"]([^'"]+)['"]/g; | ||
| const CSS_IMPORT_PATTERN = | ||
| /import\s+\w+\s+from\s+['"]([^'"]+\.tw\.css(?:\.ts|\.js)?)['"]/g; | ||
| const NAMED_CSS_IMPORT_PATTERN = | ||
| /import\s+\{([^}]+)\}\s+from\s+['"]([^'"]+\.tw\.css(?:\.ts|\.js)?)['"]/g; | ||
|
|
||
| function extractClassName(content) { | ||
| const match = | ||
| content.match(/class\s+(\w+)\s+extends\s+LightDomMixin/) || | ||
| content.match(/class\s+(\w+)\s+extends\s+ItemSectionMixin/); | ||
| return match ? match[1] : 'Unknown'; | ||
| } | ||
|
|
||
| function extractTagName(content) { | ||
| const match = content.match(/@customElement\s*\(\s*['"]([^'"]+)['"]\s*\)/); | ||
| return match ? match[1] : null; | ||
| } | ||
|
|
||
| function stripCssComments(content) { | ||
| return content.replace(/\/\*[\s\S]*?\*\//g, ''); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if content contains :host selector in CSS, excluding comments. | ||
| * Handles both inline css`` literals and .tw.css.ts module files. | ||
| */ | ||
| function containsHostSelector(content) { | ||
| const cssLiteralMatches = content.match(/css`[\s\S]*?`/g) || []; | ||
| for (const cssLiteral of cssLiteralMatches) { | ||
| const withoutComments = stripCssComments(cssLiteral); | ||
| if (HOST_SELECTOR_PATTERN.test(withoutComments)) { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // For .tw.css.ts files, check the raw content (minus comments) | ||
| const contentWithoutComments = stripCssComments(content); | ||
| if (HOST_SELECTOR_PATTERN.test(contentWithoutComments)) { | ||
| const lines = contentWithoutComments.split('\n'); | ||
| for (const line of lines) { | ||
| if (line.trim().startsWith('//')) continue; | ||
| if (HOST_SELECTOR_PATTERN.test(line)) { | ||
| return true; | ||
| } | ||
fbeaudoincoveo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| /** | ||
| * Resolves an import path to an absolute file path. | ||
| * Handles @/ aliases and relative imports. Returns null for external packages. | ||
| */ | ||
| function resolveImportPath(importPath, fromFile) { | ||
| const fromDir = dirname(fromFile); | ||
|
|
||
| if (importPath.startsWith('@/')) { | ||
| const aliasPath = importPath.replace('@/', ''); | ||
| return resolve(ATOMIC_SRC, '..', aliasPath); | ||
| } | ||
|
|
||
| if (importPath.startsWith('.')) { | ||
| const resolved = resolve(fromDir, importPath); | ||
| const extensions = ['', '.ts', '.js', '.tw.css.ts', '.tw.css.js']; | ||
| for (const ext of extensions) { | ||
| const withExt = resolved + ext; | ||
| if (existsSync(withExt)) { | ||
| return withExt; | ||
| } | ||
| } | ||
| return resolved; | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Recursively checks imported style files for :host selectors. | ||
| * Returns {hasHost, chain} where chain is the import path that led to :host. | ||
| */ | ||
| function checkImportedStyles(filePath, visited = new Set()) { | ||
| if (visited.has(filePath) || !existsSync(filePath)) { | ||
| return {hasHost: false, chain: []}; | ||
| } | ||
|
|
||
| visited.add(filePath); | ||
| const content = readFileSync(filePath, 'utf-8'); | ||
|
|
||
| if (containsHostSelector(content)) { | ||
| return {hasHost: true, chain: [filePath]}; | ||
| } | ||
|
|
||
| // Check default CSS imports | ||
| for (const [, importPath] of content.matchAll(CSS_IMPORT_PATTERN)) { | ||
| const resolvedPath = resolveImportPath(importPath, filePath); | ||
| if (resolvedPath) { | ||
| const result = checkImportedStyles(resolvedPath, visited); | ||
| if (result.hasHost) { | ||
| return {hasHost: true, chain: [filePath, ...result.chain]}; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check other imports that might be style re-exports | ||
| for (const [, importPath] of content.matchAll(IMPORT_PATTERN)) { | ||
| if (importPath.includes('.tw.css') || importPath.includes('.css')) { | ||
| const resolvedPath = resolveImportPath(importPath, filePath); | ||
| if (resolvedPath) { | ||
| const result = checkImportedStyles(resolvedPath, visited); | ||
| if (result.hasHost) { | ||
| return {hasHost: true, chain: [filePath, ...result.chain]}; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return {hasHost: false, chain: []}; | ||
| } | ||
|
|
||
| function analyzeComponent(filePath) { | ||
| const content = readFileSync(filePath, 'utf-8'); | ||
| const violations = []; | ||
|
|
||
| const isLightDomMixin = /extends\s+LightDomMixin\s*\(/.test(content); | ||
| const isItemSectionMixin = /extends\s+ItemSectionMixin\s*\(/.test(content); | ||
|
|
||
| if (!isLightDomMixin && !isItemSectionMixin) { | ||
| return violations; | ||
| } | ||
|
|
||
| const className = extractClassName(content); | ||
| const tagName = extractTagName(content); | ||
|
|
||
| // ItemSectionMixin passes styles as second argument: ItemSectionMixin(LitElement, css`...`) | ||
| if (isItemSectionMixin) { | ||
| const itemSectionMatch = content.match( | ||
| /ItemSectionMixin\s*\(\s*\w+\s*,\s*css`([\s\S]*?)`\s*\)/ | ||
| ); | ||
| if (itemSectionMatch) { | ||
| const cssContent = stripCssComments(itemSectionMatch[1]); | ||
| if (HOST_SELECTOR_PATTERN.test(cssContent)) { | ||
| violations.push({ | ||
| file: filePath, | ||
| className, | ||
| tagName, | ||
| type: 'inline', | ||
| message: `Light DOM component (via ItemSectionMixin) has ":host" selector in styles argument`, | ||
| suggestion: tagName | ||
| ? `Replace ":host" with "${tagName}" selector` | ||
| : `Replace ":host" with the component's tag name selector`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (STATIC_STYLES_PATTERN.test(content) && containsHostSelector(content)) { | ||
| violations.push({ | ||
| file: filePath, | ||
| className, | ||
| tagName, | ||
| type: 'inline', | ||
| message: `Light DOM component has inline ":host" selector in static styles`, | ||
| suggestion: tagName | ||
| ? `Replace ":host" with "${tagName}" selector` | ||
| : `Replace ":host" with the component's tag name selector`, | ||
| }); | ||
| } | ||
fbeaudoincoveo marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Check default imported style files | ||
| for (const [, importPath] of content.matchAll(CSS_IMPORT_PATTERN)) { | ||
| const resolvedPath = resolveImportPath(importPath, filePath); | ||
| if (resolvedPath) { | ||
| const result = checkImportedStyles(resolvedPath, new Set()); | ||
| if (result.hasHost) { | ||
| violations.push({ | ||
| file: filePath, | ||
| className, | ||
| tagName, | ||
| type: 'imported', | ||
| importChain: result.chain, | ||
| message: `Light DOM component imports styles containing ":host" selector`, | ||
| suggestion: `The imported style file (or its dependencies) contains ":host" which won't work in Light DOM. Either:\n - Move these styles inline and use the component tag name selector\n - Create a separate style file without ":host" for Light DOM components\n - Import chain: ${result.chain.join(' → ')}`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Check named imported style files | ||
| for (const [, namedImports, importPath] of content.matchAll( | ||
| NAMED_CSS_IMPORT_PATTERN | ||
| )) { | ||
| const resolvedPath = resolveImportPath(importPath, filePath); | ||
| if (resolvedPath) { | ||
| const result = checkImportedStyles(resolvedPath, new Set()); | ||
| if (result.hasHost) { | ||
| violations.push({ | ||
| file: filePath, | ||
| className, | ||
| tagName, | ||
| type: 'imported', | ||
| importChain: result.chain, | ||
| message: `Light DOM component imports styles (named: ${namedImports.trim()}) containing ":host" selector`, | ||
| suggestion: `The imported style file contains ":host" which won't work in Light DOM.\n - Import chain: ${result.chain.join(' → ')}`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return violations; | ||
| } | ||
|
|
||
| const EXCLUDED_FILES = [ | ||
| '.spec.ts', | ||
| '.stories.ts', | ||
| '.e2e.ts', | ||
| '.tw.css.ts', | ||
| '.d.ts', | ||
| ]; | ||
| const EXCLUDED_EXACT = ['fixture.ts', 'page-object.ts']; | ||
|
|
||
| function findTsFiles(dir, files = []) { | ||
| for (const entry of readdirSync(dir)) { | ||
| const fullPath = join(dir, entry); | ||
| const stat = statSync(fullPath); | ||
|
|
||
| if (stat.isDirectory()) { | ||
| findTsFiles(fullPath, files); | ||
| } else if (stat.isFile() && extname(entry) === '.ts') { | ||
| const shouldSkip = | ||
| EXCLUDED_FILES.some((suffix) => entry.endsWith(suffix)) || | ||
| EXCLUDED_EXACT.includes(entry); | ||
| if (!shouldSkip) { | ||
| files.push(fullPath); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return files; | ||
| } | ||
|
|
||
| function main() { | ||
| console.log('🔍 Checking for :host selectors in Light DOM components...\n'); | ||
|
|
||
| const componentsDir = join(ATOMIC_SRC, 'components'); | ||
| const files = findTsFiles(componentsDir); | ||
| const allViolations = []; | ||
|
|
||
| for (const file of files) { | ||
| allViolations.push(...analyzeComponent(file)); | ||
| } | ||
|
|
||
| if (allViolations.length === 0) { | ||
| console.log( | ||
| '✅ No :host selector violations found in Light DOM components.\n' | ||
| ); | ||
| process.exit(0); | ||
| } | ||
|
|
||
| console.log(`❌ Found ${allViolations.length} violation(s):\n`); | ||
|
|
||
| for (const violation of allViolations) { | ||
| console.log(`📁 ${violation.file}`); | ||
| console.log( | ||
| ` Component: ${violation.className} (${violation.tagName || 'no tag name'})` | ||
| ); | ||
| console.log( | ||
| ` Type: ${violation.type === 'inline' ? 'Inline styles' : 'Imported styles'}` | ||
| ); | ||
| console.log(` Issue: ${violation.message}`); | ||
| console.log(` Fix: ${violation.suggestion}`); | ||
| console.log(''); | ||
| } | ||
|
|
||
| console.log('\n💡 Why this matters:'); | ||
| console.log( | ||
| " Light DOM components (using LightDomMixin) don't have a shadow boundary." | ||
| ); | ||
| console.log( | ||
| " The :host selector targets the shadow host, which doesn't exist in Light DOM." | ||
| ); | ||
| console.log( | ||
| ' This can cause styles to leak or not apply correctly, leading to visual bugs.\n' | ||
| ); | ||
|
|
||
| process.exit(1); | ||
| } | ||
|
|
||
| main(); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.