From 71f4afbd22e00d3170e6fea951381670fc2bb51f Mon Sep 17 00:00:00 2001 From: "Pett, David" Date: Mon, 27 Apr 2026 11:21:10 -0500 Subject: [PATCH] fix(compiler): eliminate false-positive mixin warnings for barrel imports and useDefineForClassFields:false MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two unrelated conditions in `mergeExtendedClassMeta` / `resolveAndProcessExtendedClass` caused spurious build warnings for valid mixin-factory patterns. **Fix 1 — useDefineForClassFields: false** `detectModernPropDeclarations` returns `false` whenever TypeScript has stripped `PropertyDeclaration` nodes from the class body (which it always does when `useDefineForClassFields: false`). Guard the warning with a check for that flag so the "Component classes can only extend from other Stencil decorated base classes when targeting more modern JavaScript" message is never emitted in that configuration. **Fix 2 — barrel re-export support** When a mixin factory is imported through a barrel index file (`export * from './mixin'` / `export { X } from './mixin'`), the direct `statements.find()` lookup in `resolveAndProcessExtendedClass` finds nothing, triggering the "Please import class / mixin-factory declarations directly and not via barrel files" warning even for a correct setup. A new `resolveFromBarrelExports` helper follows re-exports one level deep so the real declaration is located before the warning is emitted. The dependent-class entry now records the actual source file and file name (not the barrel's) for correct downstream processing. **Test infrastructure** - `transpileModules()` added to `transpile.ts` — a multi-file variant of `transpileModule` that pre-populates `compilerCtx.moduleMap` with auxiliary source files so cross-file scenarios can be exercised in unit tests without hitting the real file system. - Three new test cases in `parse-mixin.spec.ts` covering: - `export * from` barrel re-export → no warning - named `export { X as Y } from` re-export → no warning - explicit `useDefineForClassFields: false` → no warning --- .../static-to-meta/class-extension.ts | 107 +++++++++++- .../transformers/test/parse-mixin.spec.ts | 118 ++++++++++++- src/compiler/transformers/test/transpile.ts | 158 +++++++++++++++++- 3 files changed, 372 insertions(+), 11 deletions(-) diff --git a/src/compiler/transformers/static-to-meta/class-extension.ts b/src/compiler/transformers/static-to-meta/class-extension.ts index c608f9ba3ad..36267eaa56e 100644 --- a/src/compiler/transformers/static-to-meta/class-extension.ts +++ b/src/compiler/transformers/static-to-meta/class-extension.ts @@ -1,17 +1,17 @@ -import ts from 'typescript'; import { augmentDiagnosticWithNode, buildWarn, normalizePath } from '@utils'; -import { tsResolveModuleName, tsGetSourceFile } from '../../sys/typescript/typescript-resolve-module'; +import ts from 'typescript'; + +import type * as d from '../../../declarations'; +import { tsGetSourceFile, tsResolveModuleName } from '../../sys/typescript/typescript-resolve-module'; +import { detectModernPropDeclarations } from '../detect-modern-prop-decls'; import { isStaticGetter } from '../transform-utils'; import { parseStaticEvents } from './events'; import { parseStaticListeners } from './listeners'; import { parseStaticMethods } from './methods'; import { parseStaticProps } from './props'; +import { parseStaticSerializers } from './serializers'; import { parseStaticStates } from './states'; import { parseStaticWatchers } from './watchers'; -import { parseStaticSerializers } from './serializers'; - -import type * as d from '../../../declarations'; -import { detectModernPropDeclarations } from '../detect-modern-prop-decls'; type DeDupeMember = | d.ComponentCompilerProperty @@ -48,6 +48,77 @@ const deDupeMembers = (dedupeMembers: T[], staticMembers ); }; +/** + * Attempts to find a named declaration exported from a barrel file. + * + * A barrel file is a module that only re-exports from other modules using + * `export * from '...'` or `export { X } from '...'` syntax. When a component + * imports a mixin factory from a barrel index file (e.g. `import { myFactory } + * from '../mixins'`) the compiler cannot find the declaration by scanning the + * barrel's top-level statements — it needs to follow the re-exports one level + * deep to locate the actual source file. + * + * @param config the current validated config + * @param compilerCtx the current compiler context + * @param barrelSource the source file suspected to be a barrel + * @param className the name of the declaration to find + * @returns the matched statement along with its origin source file and file name, or undefined + */ +function resolveFromBarrelExports( + config: d.ValidatedConfig, + compilerCtx: d.CompilerCtx, + barrelSource: ts.SourceFile, + className: string, +): + | { + statement: ts.ClassDeclaration | ts.FunctionDeclaration | ts.VariableStatement; + sourceFile: ts.SourceFile; + fileName: string; + } + | undefined { + for (const stmt of barrelSource.statements) { + if (!ts.isExportDeclaration(stmt) || !stmt.moduleSpecifier) { + continue; + } + + const reExportSpecifier = stmt.moduleSpecifier.getText(barrelSource).replace(/['"]/g, ''); + + // Check if this named re-export includes the class we're looking for: + // `export { className } from '...'` or `export { original as className } from '...'` + if (stmt.exportClause && ts.isNamedExports(stmt.exportClause)) { + const hasName = stmt.exportClause.elements.some((el) => { + const exportedName = el.name.text; + const localName = el.propertyName?.text ?? el.name.text; + return exportedName === className || localName === className; + }); + if (!hasName) continue; + } + // If there's no export clause it's `export * from '...'` — always follow it. + + const resolvedReExport = tsResolveModuleName(config, compilerCtx, reExportSpecifier, barrelSource.fileName); + if (!resolvedReExport?.resolvedModule) continue; + + let reExportSource: ts.SourceFile = compilerCtx.moduleMap.get( + resolvedReExport.resolvedModule.resolvedFileName, + )?.staticSourceFile; + + if (!reExportSource) { + reExportSource = tsGetSourceFile(config, resolvedReExport); + } + if (!reExportSource) continue; + + const found = reExportSource.statements.find(matchesNamedDeclaration(className)); + if (found) { + return { + statement: found, + sourceFile: reExportSource, + fileName: resolvedReExport.resolvedModule.resolvedFileName, + }; + } + } + return undefined; +} + /** * Helper function to resolve and process an extended class from a module. * This handles: @@ -108,7 +179,24 @@ function resolveAndProcessExtendedClass( } // 2) get the exported declaration from the module - const matchedStatement = foundSource.statements.find(matchesNamedDeclaration(className)); + let matchedStatement = foundSource.statements.find(matchesNamedDeclaration(className)); + // Track which source file actually contains the matched declaration. + // If the import pointed at a barrel file the real source may differ from foundSource. + let matchedSource = foundSource; + let matchedFileName = foundFile.resolvedModule.resolvedFileName; + + if (!matchedStatement) { + // The declaration wasn't found directly — the resolved module may be a barrel file that + // re-exports the class via `export * from '...'` or `export { X } from '...'` statements. + // Follow those re-exports one level deep to find the actual declaration. + const barrelResult = resolveFromBarrelExports(buildCtx.config, compilerCtx, foundSource, className); + if (barrelResult) { + matchedStatement = barrelResult.statement; + matchedSource = barrelResult.sourceFile; + matchedFileName = barrelResult.fileName; + } + } + if (!matchedStatement) { // we couldn't find the imported declaration as an exported statement in the module const err = buildWarn(buildCtx.diagnostics); @@ -135,8 +223,8 @@ function resolveAndProcessExtendedClass( // 3) if we found the class declaration, push it and check if it itself extends from another class dependentClasses.push({ classNode: foundClassDeclaration, - sourceFile: foundSource, - fileName: foundFile.resolvedModule.resolvedFileName, + sourceFile: matchedSource, + fileName: matchedFileName, }); if (keepLooking) { @@ -482,6 +570,7 @@ export function mergeExtendedClassMeta( if ( (mixinProps.length > 0 || mixinStates.length > 0) && + buildCtx.config.tsCompilerOptions?.useDefineForClassFields !== false && !detectModernPropDeclarations(extendedClass.classNode, extendedClass.sourceFile) ) { const err = buildWarn(buildCtx.diagnostics); diff --git a/src/compiler/transformers/test/parse-mixin.spec.ts b/src/compiler/transformers/test/parse-mixin.spec.ts index b2022b48dc4..a50eadb7b97 100644 --- a/src/compiler/transformers/test/parse-mixin.spec.ts +++ b/src/compiler/transformers/test/parse-mixin.spec.ts @@ -1,6 +1,6 @@ import * as ts from 'typescript'; -import { transpileModule } from './transpile'; +import { transpileModule, transpileModules } from './transpile'; // import { c, formatCode } from './utils'; describe('parse mixin', () => { @@ -146,4 +146,120 @@ describe('parse mixin', () => { }, ]); }); + + describe('mixin factory imported from a barrel index file', () => { + it('does not emit a warning when the mixin factory is re-exported via `export * from`', () => { + // The component imports the factory from a barrel index file: + // import { colorFactory } from '../mixins'; ← resolves to mixins/index.ts + // The barrel only contains `export * from './color'` — it has no direct declaration. + // Before the fix this triggered: "Unable to find 'colorFactory' in the imported module". + expect(() => + transpileModules( + { + 'component.tsx': ` + import { Component, Prop, h, Mixin } from '@stencil/core'; + import { colorFactory } from 'mixins/index.ts'; + + const ColorMixin = colorFactory; + + @Component({ tag: 'my-cmp' }) + class MyCmp extends Mixin(ColorMixin) { + @Prop() ownProp: string = 'own'; + } + `, + 'mixins/index.ts': ` + export * from './color'; + `, + 'mixins/color.ts': ` + import type { MixedInCtor } from '@stencil/core'; + + export const colorFactory = (Base: B) => { + class ColorMixin extends Base { + @Prop() color: string = 'red'; + } + return ColorMixin; + }; + `, + }, + null, + { target: ts.ScriptTarget.ES2022 }, + ), + ).not.toThrow(); + }); + + it('does not emit a warning when the mixin factory is re-exported via named `export { X } from`', () => { + expect(() => + transpileModules( + { + 'component.tsx': ` + import { Component, Prop, h, Mixin } from '@stencil/core'; + import { myColorFactory } from 'mixins/index.ts'; + + const ColorMixin = myColorFactory; + + @Component({ tag: 'my-cmp' }) + class MyCmp extends Mixin(ColorMixin) { + @Prop() ownProp: string = 'own'; + } + `, + 'mixins/index.ts': ` + export { colorFactory as myColorFactory } from './color'; + `, + 'mixins/color.ts': ` + import type { MixedInCtor } from '@stencil/core'; + + export const colorFactory = (Base: B) => { + class ColorMixin extends Base { + @Prop() color: string = 'red'; + } + return ColorMixin; + }; + `, + }, + null, + { target: ts.ScriptTarget.ES2022 }, + ), + ).not.toThrow(); + }); + + it('does not emit a warning when useDefineForClassFields is explicitly false', () => { + // When `useDefineForClassFields: false`, TypeScript does not emit class field + // initializers, so Stencil's `detectModernPropDeclarations` always returns false + // for this config. The warning must be suppressed in this case because + // `useDefineForClassFields: false` is the safe / correct Stencil configuration + // and the inheritance works correctly at runtime. + expect(() => + transpileModules( + { + 'component.tsx': ` + import { Component, Prop, h, Mixin } from '@stencil/core'; + import { colorFactory } from 'mixins/index.ts'; + + const ColorMixin = colorFactory; + + @Component({ tag: 'my-cmp' }) + class MyCmp extends Mixin(ColorMixin) { + @Prop() ownProp: string = 'own'; + } + `, + 'mixins/index.ts': ` + export * from './color'; + `, + 'mixins/color.ts': ` + import type { MixedInCtor } from '@stencil/core'; + + export const colorFactory = (Base: B) => { + class ColorMixin extends Base { + @Prop() color: string = 'red'; + } + return ColorMixin; + }; + `, + }, + null, + { target: ts.ScriptTarget.ES2022, useDefineForClassFields: false }, + ), + ).not.toThrow(); + }); + }); }); diff --git a/src/compiler/transformers/test/transpile.ts b/src/compiler/transformers/test/transpile.ts index 42400e85211..9d48ec75dea 100644 --- a/src/compiler/transformers/test/transpile.ts +++ b/src/compiler/transformers/test/transpile.ts @@ -1,5 +1,5 @@ import type * as d from '@stencil/core/declarations'; -import { mockBuildCtx, mockCompilerCtx, mockValidatedConfig } from '@stencil/core/testing'; +import { mockBuildCtx, mockCompilerCtx, mockModule, mockValidatedConfig } from '@stencil/core/testing'; import ts from 'typescript'; import { performAutomaticKeyInsertion } from '../automatic-key-insertion'; @@ -204,6 +204,162 @@ export function transpileModule( */ const prettifyTSOutput = (tsOutput: string): string => tsOutput.replace(/\s+/gm, ' '); +/** + * Multi-file variant of {@link transpileModule}. Accepts a map of `{ fileName → source }` pairs + * so tests can exercise cross-file scenarios (e.g. importing a mixin factory through a barrel + * index file). + * + * The first entry in `files` is treated as the main component file. Auxiliary files are + * registered in `compilerCtx.moduleMap` before the main file is transpiled so that + * `buildExtendsTree` can resolve them when walking import statements. + * + * @param files ordered record of { fileName: fileContent } — first entry is the main input + * @param config optional Stencil config overrides + * @param tsConfig optional TypeScript compiler option overrides + * @returns the same shape as {@link transpileModule} + */ +export function transpileModules( + files: Record, + config?: Partial | null, + tsConfig: ts.CompilerOptions = {}, +) { + const options: ts.CompilerOptions = { + ...ts.getDefaultCompilerOptions(), + allowNonTsExtensions: true, + composite: undefined, + declaration: undefined, + declarationDir: undefined, + experimentalDecorators: true, + jsx: ts.JsxEmit.React, + jsxFactory: 'h', + jsxFragmentFactory: 'Fragment', + lib: undefined, + module: ts.ModuleKind.ESNext, + noEmit: undefined, + noEmitHelpers: true, + noEmitOnError: undefined, + noLib: true, + out: undefined, + outFile: undefined, + paths: undefined, + removeComments: false, + rootDirs: undefined, + suppressOutputPathCheck: true, + target: getScriptTarget(), + types: undefined, + ...tsConfig, + }; + + const initConfig = mockValidatedConfig(); + const mergedConfig: d.ValidatedConfig = { ...initConfig, ...config }; + const compilerCtx: d.CompilerCtx = mockCompilerCtx(mergedConfig); + + // Build TS source files for every entry + const sourceFiles = new Map( + Object.entries(files).map(([fileName, src]) => [fileName, ts.createSourceFile(fileName, src, options.target)]), + ); + + // Pre-populate moduleMap with auxiliary files so buildExtendsTree can resolve them + const [mainFileName] = Object.keys(files); + for (const [fileName, sourceFile] of sourceFiles) { + if (fileName === mainFileName) continue; + const mod = mockModule({ sourceFilePath: fileName, staticSourceFile: sourceFile }); + compilerCtx.moduleMap.set(fileName, mod); + } + + const mainSourceFile = sourceFiles.get(mainFileName)!; + + let outputText: string; + let declarationOutputText: string; + + const buildCtx = mockBuildCtx(mergedConfig, compilerCtx); + + const emitCallback: ts.WriteFileCallback = (emitFilePath, data, _w, _e, tsSourceFiles) => { + if (emitFilePath.endsWith('.js')) { + outputText = prettifyTSOutput(data); + updateModule(mergedConfig, compilerCtx, buildCtx, tsSourceFiles[0], data, emitFilePath, tsTypeChecker, null); + } + if (emitFilePath.endsWith('.d.ts')) { + declarationOutputText = prettifyTSOutput(data); + } + }; + + const compilerHost: ts.CompilerHost = { + getSourceFile: (fileName) => sourceFiles.get(fileName), + writeFile: emitCallback, + getDefaultLibFileName: () => 'lib.d.ts', + useCaseSensitiveFileNames: () => false, + getCanonicalFileName: (fileName) => fileName, + getCurrentDirectory: () => '', + getNewLine: () => '', + fileExists: (fileName) => sourceFiles.has(fileName), + readFile: () => '', + directoryExists: () => true, + getDirectories: () => [], + }; + + const tsProgram = ts.createProgram([mainFileName], options, compilerHost); + const tsTypeChecker = tsProgram.getTypeChecker(); + + const transformOpts: d.TransformOptions = { + coreImportPath: '@stencil/core', + componentExport: 'lazy', + componentMetadata: null, + currentDirectory: '/', + proxy: null, + style: 'static', + styleImportData: 'queryparams', + }; + + tsProgram.emit(mainSourceFile, undefined, undefined, undefined, { + before: [ + convertDecoratorsToStatic(mergedConfig, buildCtx.diagnostics, tsTypeChecker, tsProgram), + performAutomaticKeyInsertion, + ], + after: [ + (context) => { + let newSource: ts.SourceFile; + const visitNode = (node: ts.Node): ts.Node => { + node.getSourceFile = () => newSource; + return ts.visitEachChild(node, visitNode, context); + }; + return (sf: ts.SourceFile): ts.SourceFile => { + newSource = sf; + return visitNode(sf) as ts.SourceFile; + }; + }, + convertStaticToMeta(mergedConfig, compilerCtx, buildCtx, tsTypeChecker, null, transformOpts), + ], + }); + + const moduleFile: d.Module = compilerCtx.moduleMap.get(mainFileName); + const cmps = moduleFile ? moduleFile.cmps : null; + const cmp = Array.isArray(cmps) && cmps.length > 0 ? cmps[0] : null; + const properties = cmp ? cmp.properties : null; + const states = cmp ? cmp.states : null; + const methods = cmp ? cmp.methods : null; + const isExtended = moduleFile ? moduleFile.isExtended : false; + + if (buildCtx.hasError || buildCtx.hasWarning) { + throw new Error(buildCtx.diagnostics[0].messageText as string); + } + + return { + buildCtx, + cmp, + cmps, + compilerCtx, + declarationOutputText, + diagnostics: buildCtx.diagnostics, + isExtended, + methods, + moduleFile, + outputText, + properties, + states, + }; +} + /** * Helper function for tests that converts stringified JavaScript to a runtime value. * A value from the generated JavaScript is returned based on the provided property name.