fix(compiler): copy transpileComponent output to all dist directories #10136
Conversation
Compilers using transpileComponent write directly to the filesystem with a single outputDir. After PR #10125 fixed the outputDir to use bit_roots for correct peer resolution, the compiled files were no longer available in node_modules/<pkg>. This fix copies all compiled files from the outputDir to all other dist directories (injected dirs) after transpileComponent completes, ensuring bundlers like esbuild have their output available in all required locations.
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where compiled files from compilers using transpileComponent were not available in all required dist directories. After PR #10125 changed the outputDir to use bit_roots for proper peer resolution, compiled files were only written to one location but needed to be available in multiple locations including node_modules/<pkg>.
Key Changes
- Added new method
copyCompiledFilesToOtherDists()to copy compiled files from the compiler's output directory to all other required dist directories - Integrated the copy operation into the compilation workflow by calling it after
transpileComponentcompletes
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const matches = await globby(`**/*`, { | ||
| cwd: sourceDistDirAbs, | ||
| onlyFiles: true, | ||
| ignore: ['node_modules/**'], |
There was a problem hiding this comment.
The ignore pattern uses 'node_modules/**' while the similar method copyTypesToOtherDists uses a different pattern with template literals. For consistency and to ensure the ignore pattern works correctly across different path formats, consider aligning the ignore pattern format with the existing copyTypesToOtherDists method or documenting why a different pattern is needed here.
| const matches = await globby(`**/*`, { | |
| cwd: sourceDistDirAbs, | |
| onlyFiles: true, | |
| ignore: ['node_modules/**'], | |
| const nodeModulesIgnorePattern = path.posix.join('node_modules', '**'); | |
| const matches = await globby(`**/*`, { | |
| cwd: sourceDistDirAbs, | |
| onlyFiles: true, | |
| ignore: [nodeModulesIgnorePattern], |
| await fs.ensureDir(path.dirname(dest)); | ||
| await fs.copyFile(source, dest); |
There was a problem hiding this comment.
The parent directory is created with fs.ensureDir, but if the copy operation fails (e.g., due to permission issues or disk space), there's no error handling. Consider wrapping the file copy operations in a try-catch block to handle potential I/O errors gracefully and provide meaningful error messages.
| await fs.ensureDir(path.dirname(dest)); | |
| await fs.copyFile(source, dest); | |
| try { | |
| await fs.ensureDir(path.dirname(dest)); | |
| await fs.copyFile(source, dest); | |
| } catch (err) { | |
| const error = err instanceof Error ? err : new Error(String(err)); | |
| throw new Error( | |
| `Failed to copy compiled file from "${source}" to "${dest}": ${error.message}` | |
| ); | |
| } |
| await Promise.all( | ||
| otherDirs.map(async (distDir) => { | ||
| const distDirAbs = path.join(this.workspace.path, distDir); | ||
| await Promise.all( | ||
| matches.map(async (match) => { | ||
| const source = path.join(sourceDistDirAbs, match); | ||
| const dest = path.join(distDirAbs, match); | ||
| await fs.ensureDir(path.dirname(dest)); | ||
| await fs.copyFile(source, dest); | ||
| }) | ||
| ); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
The nested Promise.all calls could potentially create a large number of concurrent file operations. For workspaces with many dist directories and files, this could lead to resource exhaustion or file descriptor limits being exceeded. Consider using a concurrency limiter like p-limit to control the number of simultaneous file copy operations.
| async copyCompiledFilesToOtherDists() { | ||
| const distDirs = await this.distDirs(); | ||
| if (distDirs.length <= 1) return; | ||
|
|
||
| // The compiler writes to the outputDir from getComponentPackagePath + distDirName | ||
| const outputDir = await this.workspace.getComponentPackagePath(this.component); | ||
| const distDirName = this.compilerInstance.getDistDir?.() || DEFAULT_DIST_DIRNAME; | ||
| const sourceDistDirAbs = path.join(outputDir, distDirName); | ||
|
|
||
| // Check if the dist directory exists (compiler may not have written anything) | ||
| if (!(await fs.pathExists(sourceDistDirAbs))) return; | ||
|
|
||
| // Find which distDir corresponds to the outputDir and get the others | ||
| const outputDirRelative = path.relative(this.workspace.path, outputDir); | ||
| const sourceDistDirRelative = path.join(outputDirRelative, distDirName); | ||
| const otherDirs = distDirs.filter((dir) => dir !== sourceDistDirRelative); | ||
|
|
||
| if (!otherDirs.length) return; | ||
|
|
||
| const matches = await globby(`**/*`, { | ||
| cwd: sourceDistDirAbs, | ||
| onlyFiles: true, | ||
| ignore: ['node_modules/**'], | ||
| }); | ||
| if (!matches.length) return; | ||
|
|
||
| await Promise.all( | ||
| otherDirs.map(async (distDir) => { | ||
| const distDirAbs = path.join(this.workspace.path, distDir); | ||
| await Promise.all( | ||
| matches.map(async (match) => { | ||
| const source = path.join(sourceDistDirAbs, match); | ||
| const dest = path.join(distDirAbs, match); | ||
| await fs.ensureDir(path.dirname(dest)); | ||
| await fs.copyFile(source, dest); | ||
| }) | ||
| ); | ||
| }) | ||
| ); | ||
| } |
There was a problem hiding this comment.
The method copyCompiledFilesToOtherDists lacks test coverage for the new functionality. Given that similar methods like copyTypesToOtherDists exist in the codebase and this is critical functionality for ensuring files are available in all required locations, test coverage should be added to verify the copying behavior works correctly across different scenarios (e.g., multiple dist dirs, missing source dir, file copy failures).
Compilers using
transpileComponentwrite directly to the filesystem with a singleoutputDir. After PR #10125 fixed theoutputDirto usebit_rootsfor correct peer resolution, the compiled files were no longer available innode_modules/<pkg>.This fix copies all compiled files from the
outputDirto all other dist directories (injected dirs) aftertranspileComponentcompletes, ensuring bundlers like esbuild have their output available in all required locations.