Closed mt3o closed 2 months ago
Hey @mt3o 👋
While I can see how this could be an issue, some of these reproduction steps are too vague for us to be able to move forward with this. While we don't want you to share any work you're not permitted to, we do need some type of reproduction case here to clarify things like:
Otherwise, we could spend quite a bit of time building something that doesn't actually reproduce the problem you're seeing here.
One possibility, would be to use the stencil component starter (npm init stencil@latest component
) to build a component library, installing its dependencies, then running the generate command (npm run generate
) build out a bunch of components here.
Another would be to use the Ionic Framework (built with Stencil) and try to use it and its stencil config to try to reproduce the problem.
Thanks for the issue! This issue has been labeled as needs reproduction
. This label is added to issues that need a code reproduction.
Please reproduce this issue in an Stencil starter component library and provide a way for us to access it (GitHub repo, StackBlitz, etc). Without a reliable code reproduction, it is unlikely we will be able to resolve the issue, leading to it being closed.
If you have already provided a code snippet and are seeing this message, it is likely that the code snippet was not enough for our team to reproduce the issue.
For a guide on how to create a good reproduction, see our Contributing Guide.
Hi Unfortunately i'm unable to reproduce the issue in a fresh small repo. Race conditions are usually difficult to reproduce ;-) I will try again later today, scripting to create more components. But I can share the config.
import {sass} from '@stencil/sass';
import type {Config} from '@stencil/core';
import {inlineSvg} from 'stencil-inline-svg';
import { rollapVisualizerPluginConfig } from './project-configuration-extensions/project-configuration-extensions-configs';
import { configurationExtensionsLoader } from './project-configuration-extensions/project-configuration-extensions-loader';
import {getGitHash} from './src/getGitHash'
const parseBoolean = (value: string|undefined, fallback: boolean) => {
if(value===undefined) return fallback;
if (['true','1','on','enabled'].includes(value.toLowerCase())) return true;
if (['false','0','off','disabled'].includes(value.toLowerCase())) return false;
return fallback;
}
// *my workaround for the race condition, on ci/cd its set to true*
const stencilNoCopy = parseBoolean(process.env.STENCIL_NO_COPY, false)
const copyDist = stencilNoCopy ? [] : [
// {src: '../dist/esm', dest: 'esm'},
{src: '../dist/lazy-loader', dest: 'lazy-loader'},
{src: '../dist/custom-elements', dest: 'custom-elements'},
{src: '../dist/ui', dest: 'ui'},
{src: '../dist/types', dest: 'types'},
]
const copy = [
...copyDist,
{src: '../node_modules/@xx/fonts/css', dest: 'fonts/css'},
{src: '../node_modules/@xx/fonts/fonts', dest: 'fonts/fonts'},
{src: '../node_modules/@xx/icons/css', dest: 'icons/css'},
{src: '../node_modules/@xx/icons/fonts', dest: 'icons/fonts'},
{src: '../node_modules/@xx/web-ui', dest: 'web-ui'},
{src: 'demo-pages', dest: 'demo-pages'},
];
// noinspection JSUnusedGlobalSymbols
export const config: Config = {
namespace: 'ui',
devServer: {
openBrowser: false,
},
autoprefixCss: true,
taskQueue: 'async',
minifyJs: parseBoolean(process.env['MINIFY_JS'],true),
minifyCss: parseBoolean(process.env['MINIFY_CSS'],true),
excludeUnusedDependencies: true,
extras: {
appendChildSlotFix: true,
cloneNodeFix: true,
enableImportInjection: true,
},
validateTypes: true,
validatePrimaryPackageOutputTarget: true,
hashFileNames: false,
outputTargets: [
{
type: 'dist',
esmLoaderPath: 'loader',
},
{
type: "dist-custom-elements",
customElementsExportBehavior: 'bundle',
isPrimaryPackageOutputTarget: true,
minify: true,
dir: 'dist/custom-elements',
},
{
type: 'stats',
},
{
type: 'dist-lazy',
dir: 'dist/lazy-loader',
cjsDir:'dist-lazy-loader-cjs',
esmDir: 'dist-lazy-loader-esm',
empty: true,
},
{
type: 'docs-readme',
},
{
type: 'docs-json',
file: 'documentation.json'
},
{
type: 'docs-vscode',
file: 'custom-elements.json',
},
{
type: 'dist-hydrate-script',
},
{
type: 'www',
baseUrl: 'https://www.se.com/',
copy,
serviceWorker: null, // disable service workers
},
],
testing: {
verbose: true,
browserArgs: ['--no-sandbox', '--disable-setuid-sandbox'],
transform: {
'^.+\\.svg$': '<rootDir>/svgTransform.js',
},
transformIgnorePatterns: ['/node_modules/(?!@xx/icons.svg)'],
roots: ['<rootDir>'],
collectCoverage: true,
//uncomment if you want to see the browser during tests
// browserHeadless: false,
// browserDevtools: true,
// browserSlowMo: 1000, //milliseconds
},
preamble: 'UI script \ncompilation date: ' + Date()+'\ncommit: '+ getGitHash(),
plugins: [
sass({
injectGlobalPaths: [
'node_modules/@xx/web-ui/styles/_theme.scss',
'./src/styles/global.scss',
'./src/styles/variables.scss',
'./src/styles/mixins/index.scss',
],
}),
inlineSvg(),
...configurationExtensionsLoader(rollapVisualizerPluginConfig)
],
};
I finally created the repo to present the problem so that you can replicate.
https://github.com/mt3o/stencil-race-condition-between-copy-and-build
https://stackblitz.com/~/github.com/mt3o/stencil-race-condition-between-copy-and-build
Please verify
Hey @mt3o,
Thanks for the repro! Can you provide instructions for using it to verify this bug? I see a generate-scripts.mjs
file in this directory, is that supposed to be be used in some way? Should we run the build
script before/after this is run (if at all)?
Thanks!
No, you don't have to do anything. Just npm install && npm start
. I left the generate-scripts.mjs
file so that the experiment could be recreated, and more files could be created to make the stencil compilation to take even longer.
If you open StackBlitz link you should see this:
It fails, because it tries to copy files that don't exist. Yet.
Please note lines 22, 23, 24 in stencil.config.ts where there is the task to the not-yet-existing copy files. Files that are supposed to be built during the compilation. The npm start
failed because the copy task expects compilation output to be present, and fails because it's not there. This is the race condition I tried to explain in the beginning. Copy happens faster than the compilation, so if you copy the file that is to be created during the compilation - the operation fails.
If you comment out these lines, disabling the copying, running npm start
ends with success. Avoiding the attempt to copy the compilation output - allows the compilation to be processed successfully.
What's interesting, if you uncomment those lines, and rerun the compilation - it ends with success. The reason is simple - now past compilation artifacts exist, can be copied - so there is no error, and the compilation eventually gets done. What's annoying, if you - the frontend developer - alter your code, first attempt to build - will reuse your old files. Rerunning the build process will copy proper files.
@mt3o thanks, I've ingested this issue into our backlog. At this point I can't provide any estimate as to when we get the chance to look into it. We appreciate any help on the issue and are available for questions as they arise.
@christian-bromann
The fix would be, in file https://github.com/ionic-team/stencil/blob/main/src/compiler/output-targets/index.ts#L30
to alter the block:
await Promise.all([
//outputCopy(config, compilerCtx, buildCtx), //remove this one
outputCollection(config, compilerCtx, buildCtx, changedModuleFiles),
outputCustomElements(config, compilerCtx, buildCtx),
outputHydrateScript(config, compilerCtx, buildCtx),
outputLazyLoader(config, compilerCtx),
outputLazy(config, compilerCtx, buildCtx),
]);
//and call it after compilation is done here, so that all files are already in place
await outputCopy(config, compilerCtx, buildCtx)
await outputWww(config, compilerCtx, buildCtx);
@mt3o thanks, mind raising a PR for this? The team would greatly appreciate any help.
I've raised a PR for this in #5902
Prerequisites
Stencil Version
4.13.0
Current Behavior
When building with stencil, when the copy task is issued here: https://github.com/ionic-team/stencil/blob/main/src/compiler/output-targets/index.ts#L30
there is
Promise.all
calling all tasks, including copy tasks, and regular building tasks. When the build (compilation, regular targets) takes longer, copy task is executed first, trying to copy non-existing directories. It's a race condition between compilation and copying.I have problems replicating the issue with a smaller repo, and obviously, I can't share my work project. I'm guessing the issue happens only in big projects, or with poor hardware, as it's a race it depends on the performance.
For me - the workaround is to either create empty directories first (risking incomplete build), or not use copy-task at all, and instead copy files in standalone CI/CD script. On developer machine, with watch mode, the problem is nearly nonexistent, as there are some files in /dist all the time. If something doesn't work, devs just restart the build script and then it goes smoothly. Such behavior in CI/CD is difficult to achieve.
Expected Behavior
Please consider moving
await outputCopy(config, compilerCtx, buildCtx),
after thePromise.all
and before the www target. This should solve the race problem.System Info
Steps to Reproduce
Set up a big repo with complex components, Add lots of targets to be compiled Set www target to be built Have all targets be copied into www run
npx stencil build
Stencil runs all tasks as in Promise.all call, copy ends after build tasks are over, compilation breaks with error that some dirs don't exist in /dist.
Code Reproduction URL
no url :(
Additional Information
The log is as follows, it's that the clear copy task is ending before the build tasks are finished. So I'm concluding that moving the copy task out of
Promise.all
will solve the problem - because the copy task will be executed only after all build tasks are done.