From e6369c9eaf3d1303b272df538c45691c95428c77 Mon Sep 17 00:00:00 2001 From: Nicolas Stepien Date: Fri, 21 Nov 2025 21:45:47 +0000 Subject: [PATCH 1/2] refactor: dedupe and optimize parsing work --- .gitignore | 2 +- README.md | 1 + src/index.ts | 319 +++++++++++---------------- test/fixtures/comprehensive.input.ts | 3 +- test/fixtures/fake.ts | 4 + test/fixtures/imported-style.ts | 2 + test/fixtures/no-ecij.input.ts | 6 +- test/plugin.test.ts | 12 +- 8 files changed, 154 insertions(+), 195 deletions(-) diff --git a/.gitignore b/.gitignore index d4d011a..2a01fd1 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,4 @@ /.cache +/coverage /dist /node_modules -/coverage diff --git a/README.md b/README.md index 9232ff1..5f10f65 100644 --- a/README.md +++ b/README.md @@ -170,6 +170,7 @@ npm test -- -u ## TODO +- Log CSS extraction failures - Scope handling - Validate that the `css` used refers to the ecij export - Full import/export handling (default/namespace import/export) diff --git a/src/index.ts b/src/index.ts index 0a98a8d..ec7b41b 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,12 +29,21 @@ export interface Configuration { } interface Declaration { - index: number; - varName: string | undefined; + className: string; node: TaggedTemplateExpression; hasInterpolations: boolean; } +interface ParsedFileInfo { + declarations: Declaration[]; + localIdentifiers: ReadonlyMap; + importedIdentifiers: ReadonlyMap< + string, + { source: string; imported: string } + >; + exportNameToValueMap: ReadonlyMap; +} + // allow .js, .cjs, .mjs, .ts, .cts, .mts, .jsx, .tsx files const JS_TS_FILE_REGEX = /\.[cm]?[jt]sx?$/; @@ -71,76 +80,64 @@ export function ecij({ exclude = [NODE_MODULES_REGEX, D_TS_FILE_REGEX], classPrefix = 'css-', }: Configuration = {}): Plugin { + const parsedFileInfoCache = new Map(); + // Map to store extracted CSS content per source file // Key: virtual module ID, Value: css content const extractedCssPerFile = new Map(); - // Cache for resolved imported class names - const importedClassNameCache = new Map(); - /** - * Generates a consistent, unique class name based on file path and variable name or index + * Parses a file and extracts all relevant information in a single pass */ - function generateClassName( + async function parseFile( + context: TransformPluginContext, filePath: string, - index: number, - variableName: string | undefined, - ): string { + code?: string, + ): Promise { + // The code loaded from `readFile` might not be identical + // to the code passed in after it has been processed by other plugins, + // as such we cannot rely on the position of declarations being the same, + // and the new code should be parsed. + if (code === undefined && parsedFileInfoCache.has(filePath)) { + return parsedFileInfoCache.get(filePath)!; + } + // Convert absolute path to project-relative path and normalize to Unix format // to ensure consistent hashes across different build environments (Windows/Unix) const relativePath = relative(PROJECT_ROOT, filePath).replaceAll('\\', '/'); - // Create a hash from the relative file path, index, - // and identifier for consistency across builds. - // The index is always used to avoid collisions with other variables - // with the same name in the same file. - const hash = hashText(`${relativePath}:${index}:${variableName}`); - - return `${classPrefix}${hash}`; - } - - /** - * Resolves an imported class name by reading and processing the source file - */ - async function resolveImportedClassName( - context: TransformPluginContext, - importerPath: string, - importSource: string, - exportedName: string, - ): Promise { - // Resolve the import path relative to the importer - const resolvedId = await context.resolve(importSource, importerPath); - - if (resolvedId == null) return; - - const resolvedPath = resolvedId.id; + // Read the source file + const sourceText = + code ?? (await context.fs.readFile(filePath, { encoding: 'utf8' })); - // Check cache - const cacheKey = `${resolvedPath}:${exportedName}`; + const parseResult = parseSync(filePath, sourceText); + const importedIdentifiers = new Map< + string, + { source: string; imported: string } + >(); - if (importedClassNameCache.has(cacheKey)) { - // TODO: turn this map into a map of maps (filePath => name => className) - // so we can avoid computing the same file multiple times. - // We should also cache the results of `extractCssFromCode`. - return importedClassNameCache.get(cacheKey)!; + // Collect imports + for (const staticImport of parseResult.module.staticImports) { + for (const entry of staticImport.entries) { + // TODO: support default and namespace imports + if (entry.importName.kind === 'Name') { + importedIdentifiers.set(entry.localName.value, { + source: staticImport.moduleRequest.value, + imported: entry.importName.name!, + }); + } + } } - // Read the source file - const sourceCode = await context.fs.readFile(resolvedPath, { - encoding: 'utf8', - }); - - // Parse to find the exported variable - const parseResult = parseSync(resolvedPath, sourceCode); - const localNameToExportedNameMap = new Map(); + // Collect exports for (const staticExport of parseResult.module.staticExports) { for (const entry of staticExport.entries) { // TODO: handle re-exports if (entry.importName.kind !== 'None') continue; - // TODO: handle other export types (default, *) + // TODO: support default and namespace exports if ( entry.exportName.kind === 'Name' && entry.localName.kind === 'Name' @@ -153,25 +150,52 @@ export function ecij({ } const declarations: Declaration[] = []; + const localIdentifiers = new Map(); + const exportNameToValueMap = new Map(); const taggedTemplateExpressionFromVariableDeclarator = new Set(); + function recordIdentifierWithValue(localName: string, value: string) { + localIdentifiers.set(localName, value); + + if (localNameToExportedNameMap.has(localName)) { + const exportedName = localNameToExportedNameMap.get(localName)!; + exportNameToValueMap.set(exportedName, value); + } + } + function handleTaggedTemplateExpression( - varName: string | undefined, + localName: string | undefined, node: TaggedTemplateExpression, ) { - if (node.tag.type === 'Identifier' && node.tag.name === 'css') { - // Store info about this declaration - declarations.push({ - index: declarations.length, - varName, - node, - hasInterpolations: node.quasi.expressions.length > 0, - }); + if (!(node.tag.type === 'Identifier' && node.tag.name === 'css')) { + return; + } + + const index = declarations.length; + + // Create a hash from the relative file path, index, + // and identifier for consistency across builds. + // The index is always used to avoid collisions with other variables + // with the same name in the same file. + const hash = hashText(`${relativePath}:${index}:${localName}`); + + const className = `${classPrefix}${hash}`; + + declarations.push({ + className, + node, + hasInterpolations: node.quasi.expressions.length > 0, + }); + + // Record generated class names for css declarations + if (localName !== undefined) { + recordIdentifierWithValue(localName, className); } } + // Visit AST to collect declarations and literal values const visitor = new Visitor({ VariableDeclarator(node) { if (node.init === null || node.id.type !== 'Identifier') return; @@ -186,50 +210,36 @@ export function ecij({ (typeof node.init.value === 'string' || typeof node.init.value === 'number') ) { - const exportedName = localNameToExportedNameMap.get(localName); - - if (exportedName === undefined) return; - - const cacheKey = `${resolvedPath}:${exportedName}`; - importedClassNameCache.set(cacheKey, String(node.init.value)); + const value = String(node.init.value); + recordIdentifierWithValue(localName, value); } }, + TaggedTemplateExpression(node) { - if (taggedTemplateExpressionFromVariableDeclarator.has(node)) { - return; + if (!taggedTemplateExpressionFromVariableDeclarator.has(node)) { + // No variable name for inline expressions + handleTaggedTemplateExpression(undefined, node); } - - // No variable name for inline expressions - handleTaggedTemplateExpression(undefined, node); }, }); visitor.visit(parseResult.program); - for (const declaration of declarations) { - const localName = declaration.varName; - - if (localName === undefined) continue; - - const exportedName = localNameToExportedNameMap.get(localName); - - if (exportedName === undefined) continue; + const parsedInfo: ParsedFileInfo = { + declarations, + localIdentifiers, + importedIdentifiers, + exportNameToValueMap, + }; - const cacheKey = `${resolvedPath}:${exportedName}`; - const className = generateClassName( - resolvedPath, - declaration.index, - localName, - ); - importedClassNameCache.set(cacheKey, className); - } + parsedFileInfoCache.set(filePath, parsedInfo); - return importedClassNameCache.get(cacheKey); + return parsedInfo; } /** * Extracts CSS from template literals in the source code using AST parsing - * Supports interpolations of class names (both local and imported) + * Supports interpolations of strings and numbers (both local and imported) */ async function extractCssFromCode( context: TransformPluginContext, @@ -241,6 +251,9 @@ export function ecij({ cssContent: string; hasUnprocessedCssBlocks: boolean; }> { + const { declarations, localIdentifiers, importedIdentifiers } = + await parseFile(context, filePath, code); + const cssExtractions: Array<{ className: string; cssContent: string; @@ -251,126 +264,52 @@ export function ecij({ end: number; className: string; }> = []; - // Maps variable name to generated class name - const localClassNames = new Map(); - // Maps imported identifier to source file - const imports = new Map< - string, - { - source: string; - imported: string; - } - >(); - // Helper to resolve a class name from an identifier - function resolveClassName( + // Helper to resolve a value from an identifier + async function resolveValue( identifierName: string, - ): string | undefined | Promise { - // Check if it's a local class name - if (localClassNames.has(identifierName)) { - return localClassNames.get(identifierName)!; + ): Promise { + // Check if it's a local identifier + if (localIdentifiers.has(identifierName)) { + return localIdentifiers.get(identifierName)!; } - // Check if it's an imported class name - if (imports.has(identifierName)) { - const { source, imported } = imports.get(identifierName)!; - return resolveImportedClassName(context, filePath, source, imported); - } + // Check if it's an imported identifier + if (importedIdentifiers.has(identifierName)) { + const { source, imported } = importedIdentifiers.get(identifierName)!; - return undefined; - } + // Resolve the import path relative to the importer + const resolvedId = await context.resolve(source, filePath); - // Parse the code into an AST - const parseResult = parseSync(filePath, code); + if (resolvedId != null) { + const { exportNameToValueMap } = await parseFile( + context, + resolvedId.id, + ); - // First pass: collect import declarations - for (const staticImport of parseResult.module.staticImports) { - for (const entry of staticImport.entries) { - // TODO: support default and namespace imports - if (entry.importName.kind === 'Name') { - imports.set(entry.localName.value, { - source: staticImport.moduleRequest.value, - imported: entry.importName.name!, - }); + return exportNameToValueMap.get(imported); } } - } - - const declarations: Declaration[] = []; - - const taggedTemplateExpressionFromVariableDeclarator = - new Set(); - function handleTaggedTemplateExpression( - varName: string | undefined, - node: TaggedTemplateExpression, - ) { - if (node.tag.type === 'Identifier' && node.tag.name === 'css') { - // Store info about this declaration - declarations.push({ - index: declarations.length, - varName, - node, - hasInterpolations: node.quasi.expressions.length > 0, - }); - } + return; } - // Do the tracking pass first to build localClassNames map - const visitor = new Visitor({ - VariableDeclarator(node) { - if (node.init === null || node.id.type !== 'Identifier') return; - - const localName = node.id.name; - - if (node.init.type === 'TaggedTemplateExpression') { - taggedTemplateExpressionFromVariableDeclarator.add(node.init); - handleTaggedTemplateExpression(localName, node.init); - } else if ( - node.init.type === 'Literal' && - (typeof node.init.value === 'string' || - typeof node.init.value === 'number') - ) { - localClassNames.set(localName, String(node.init.value)); - } - }, - TaggedTemplateExpression(node) { - if (taggedTemplateExpressionFromVariableDeclarator.has(node)) { - return; - } - - // No variable name for inline expressions - handleTaggedTemplateExpression(undefined, node); - }, - }); - - visitor.visit(parseResult.program); - // Helper to add a processed CSS declaration function addProcessedDeclaration( declaration: Declaration, cssContent: string, ) { - const className = generateClassName( - filePath, - declaration.index, - declaration.varName, - ); - - // Only store in localClassNames if it has a variable name - if (declaration.varName) { - localClassNames.set(declaration.varName, className); - } + const { className, node } = declaration; cssExtractions.push({ className, cssContent: cssContent.trim(), - sourcePosition: declaration.node.start, + sourcePosition: node.start, }); replacements.push({ - start: declaration.node.start, - end: declaration.node.end, + start: node.start, + end: node.end, className, }); } @@ -406,15 +345,15 @@ export function ecij({ } const identifierName = expression.name; - const resolvedClassName = await resolveClassName(identifierName); + const resolvedValue = await resolveValue(identifierName); - if (resolvedClassName === undefined) { + if (resolvedValue === undefined) { // Cannot resolve - skip this entire css`` block allResolved = false; break; } - cssContent += resolvedClassName; + cssContent += resolvedValue; } } @@ -473,10 +412,10 @@ export function ecij({ return { name: 'ecij', - buildStart() { - // Clear the cache when the server restarts + buildEnd() { + // Clear caches between builds + parsedFileInfoCache.clear(); extractedCssPerFile.clear(); - importedClassNameCache.clear(); }, resolveId(id) { diff --git a/test/fixtures/comprehensive.input.ts b/test/fixtures/comprehensive.input.ts index ddcee7d..5ee02c1 100644 --- a/test/fixtures/comprehensive.input.ts +++ b/test/fixtures/comprehensive.input.ts @@ -1,5 +1,5 @@ import { css } from 'ecij'; -import { bgColor as background, redClass } from './imported-style'; +import { bgColor as background, redClass, width } from './imported-style'; // Basic CSS transformation export const buttonClass = css` @@ -31,6 +31,7 @@ const highlightedClass = css` export const importedClass = css` /* imported */ background: ${background}; + width: ${width}px; &.${redClass} { border-color: red; diff --git a/test/fixtures/fake.ts b/test/fixtures/fake.ts index 837db1b..f0a057b 100644 --- a/test/fixtures/fake.ts +++ b/test/fixtures/fake.ts @@ -1,3 +1,7 @@ export function css(_: TemplateStringsArray) { return ''; } + +export function unrelated(_: TemplateStringsArray) { + return ''; +} diff --git a/test/fixtures/imported-style.ts b/test/fixtures/imported-style.ts index 82758f3..11712e3 100644 --- a/test/fixtures/imported-style.ts +++ b/test/fixtures/imported-style.ts @@ -2,6 +2,8 @@ import { css } from 'ecij'; const red = 'red'; +export const width = 40.123; + const backgroundColor = 'white'; export const redClass = css` diff --git a/test/fixtures/no-ecij.input.ts b/test/fixtures/no-ecij.input.ts index cb590fa..8f950c2 100644 --- a/test/fixtures/no-ecij.input.ts +++ b/test/fixtures/no-ecij.input.ts @@ -1,6 +1,10 @@ // File without any ecij usage -import { css } from './fake'; +import { css, unrelated } from './fake'; +// ignore unrelated tagged template literals +export const unknown = unrelated`this is not css`; + +// Ignore non-ecij css tag functions export const buttonClass = css` color: blue; padding: 10px; diff --git a/test/plugin.test.ts b/test/plugin.test.ts index b88c98b..b9a6035 100644 --- a/test/plugin.test.ts +++ b/test/plugin.test.ts @@ -73,6 +73,7 @@ test('comprehensive CSS-in-JS patterns', async () => { .css-873c0af7 { /* imported */ background: white; + width: 40.123px; &.css-348273b1 { border-color: red; @@ -129,11 +130,18 @@ test.fails('ignore non-ecij css tag functions', async () => { const result = await buildWithPlugin(fixturePath); expect(result.js).toMatchInlineSnapshot(` - "//#region test/fixtures/no-ecij.input.ts + "//#region test/fixtures/fake.ts + function unrelated(_) { + return ""; + } + + //#endregion + //#region test/fixtures/no-ecij.input.ts + const unknown = unrelated\`this is not css\`; const buttonClass = "css-25e9670b"; //#endregion - export { buttonClass };" + export { buttonClass, unknown };" `); // No CSS should be generated From fb82f88514594061167385bdd36bf99cfe3927fe Mon Sep 17 00:00:00 2001 From: Nicolas Stepien Date: Fri, 21 Nov 2025 21:51:48 +0000 Subject: [PATCH 2/2] test unresolvable identifier in interpolation --- test/fixtures/complex-interpolation.input.ts | 8 ++++++++ test/plugin.test.ts | 5 ++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/test/fixtures/complex-interpolation.input.ts b/test/fixtures/complex-interpolation.input.ts index 0e2dae8..4ae89c2 100644 --- a/test/fixtures/complex-interpolation.input.ts +++ b/test/fixtures/complex-interpolation.input.ts @@ -5,3 +5,11 @@ export const dynamicClass = css` color: ${Math.random() > 0.5 ? 'red' : 'blue'}; padding: 10px; `; + +// This has an identifier that cannot be resolved statically +export const unresolvedIdentifierClass = css` + color: ${ + // @ts-expect-error + unknownVariable + }; +`; diff --git a/test/plugin.test.ts b/test/plugin.test.ts index b9a6035..f8df178 100644 --- a/test/plugin.test.ts +++ b/test/plugin.test.ts @@ -166,9 +166,12 @@ test('skip css blocks with complex interpolations', async () => { color: \${Math.random() > .5 ? "red" : "blue"}; padding: 10px; \`; + const unresolvedIdentifierClass = css\` + color: \${unknownVariable}; + \`; //#endregion - export { dynamicClass };" + export { dynamicClass, unresolvedIdentifierClass };" `); // CSS blocks with complex expressions are skipped