Skip to content
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

bug: Copy task runs in parallel to build tasks creating a race condition #5592

Open
3 tasks done
mt3o opened this issue Mar 26, 2024 · 5 comments
Open
3 tasks done

bug: Copy task runs in parallel to build tasks creating a race condition #5592

mt3o opened this issue Mar 26, 2024 · 5 comments
Assignees
Labels
Awaiting Reply This PR or Issue needs a reply from the original reporter.

Comments

@mt3o
Copy link

mt3o commented Mar 26, 2024

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 the Promise.all and before the www target. This should solve the race problem.

System Info

System: node 18.16.1
    Platform: windows (10.0.19045)
   CPU Model: 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz (16 cpus)
    Compiler: C:\foobar\node_modules\@stencil\core\compiler\stencil.js
       Build: 1710170895
     Stencil: 4.12.6
  TypeScript: 5.3.3
      Rollup: 2.56.3
      Parse5: 7.1.2
      jQuery: 4.0.0-pre
      Terser: 5.29.1

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.

[58:20.3]  create workers, maxWorkers: 7  MEM: 142.1MB
[58:20.3]  Starting compilation in watch mode...  MEM: 142.1MB
[58:24.3]  build, search, dev mode, started ...
[58:24.3]  start build, 2024-03-26T17:58:24  MEM: 453.9MB
[58:24.3]  cleaning 6 dirs ...  MEM: 453.9MB
[58:24.3]  cleaning dirs finished in 2 ms  MEM: 454.0MB
[58:24.4]  transpile started ...
[58:32.5]  Transpiled modules: [ ... ]

[58:32.5]  generated app types started ...  MEM: 744.0MB
[58:32.5]  generated app types finished: src/components.d.ts in 22 ms  MEM: 747.4MB
[58:32.5]  transpile finished in 8.18 s
[58:32.5]  generate outputs started ...  MEM: 747.6MB
[58:32.5]  getComponentAssetsCopyTasks: 8  MEM: 747.6MB
[58:32.5]  getComponentAssetsCopyTasks: 8  MEM: 747.6MB
[58:32.5]  copy started ...
[58:32.5]  generate hydrate app started ...
[58:32.5]  generate lazy + source maps started ...
[58:32.5]  generateEntryModules, 144 entryModules  MEM: 747.7MB
[58:36.4]  copy finished (862 files) in 3.84 s
[59:02.7]  generate hydrate app finished in 30.14 s
[59:06.9]  generate lazy + source maps finished in 34.35 s
[59:06.9]  generate www started ...  MEM: 2443.5MB
[59:06.9]  generateIndexHtml, write: www/index.html  MEM: 2443.5MB
[59:06.9]  generate www finished in 6 ms  MEM: 2443.5MB
[59:07.1]  updated readme docs: (...)
[59:07.1]  updated readme docs: (...)
[59:07.1]  updated readme docs: (...)
[59:07.1]  generate types started ...  MEM: 2451.7MB
[59:07.1]  generate types finished in 1 ms  MEM: 2451.7MB
[59:07.1]  generate outputs finished in 34.55 s  MEM: 2451.7MB
[59:07.1]  aborted build, 42750ms  MEM: 2451.8MB

[ ERROR ]  ENOENT: no such file or directory, stat 'C:\foobar\dist\lazy-loader'

[ ERROR ]  ENOENT: no such file or directory, stat 'C:\foobar\dist\custom-elements'

[ ERROR ]  ENOENT: no such file or directory, stat 'C:\foobar\dist\types'

[ ERROR ]  ENOENT: no such file or directory, stat 'C:\foobar\dist\search'
@ionitron-bot ionitron-bot bot added the triage label Mar 26, 2024
@rwaskiewicz rwaskiewicz self-assigned this Mar 26, 2024
@rwaskiewicz
Copy link
Member

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:

  1. Output targets used
  2. How each output target is configured

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.

@rwaskiewicz rwaskiewicz added the ionitron: needs reproduction This PR or Issue does not have a reproduction case URL label Mar 26, 2024
@rwaskiewicz rwaskiewicz removed their assignment Mar 26, 2024
Copy link

ionitron-bot bot commented Mar 26, 2024

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.

@ionitron-bot ionitron-bot bot removed the triage label Mar 26, 2024
@mt3o
Copy link
Author

mt3o commented Mar 27, 2024

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)
  ],
};

@mt3o
Copy link
Author

mt3o commented Apr 23, 2024

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

@rwaskiewicz rwaskiewicz added triage and removed ionitron: needs reproduction This PR or Issue does not have a reproduction case URL labels Apr 25, 2024
@rwaskiewicz rwaskiewicz self-assigned this May 1, 2024
@rwaskiewicz
Copy link
Member

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!

@rwaskiewicz rwaskiewicz added the Awaiting Reply This PR or Issue needs a reply from the original reporter. label May 1, 2024
@ionitron-bot ionitron-bot bot removed the triage label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Reply This PR or Issue needs a reply from the original reporter.
Projects
None yet
Development

No branches or pull requests

2 participants