Skip to content

Conversation

@Janpot
Copy link
Member

@Janpot Janpot commented Dec 14, 2021

Investigating these build failures: https://github.com/mui-org/material-ui/runs/4516977408?check_suite_focus=true

buildTypes

Builds a project with a fix for
https://github.com/microsoft/TypeScript/issues/39117

Options:
  --help  Show help                                                    [boolean]

Error: Command failed: yarn tsc -b /home/runner/work/material-ui/material-ui/packages/mui-joy/tsconfig.build.json
error Command failed with exit code 2.

    at ChildProcess.exithandler (child_process.js:383:12)
    at ChildProcess.emit (events.js:400:28)
    at maybeClose (internal/child_process.js:1058:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:293:5) {
  killed: false,
  code: 2,
  signal: null,
  cmd: 'yarn tsc -b /home/runner/work/material-ui/material-ui/packages/mui-joy/tsconfig.build.json',
  stdout: '$ /home/runner/work/material-ui/material-ui/node_modules/.bin/tsc -b /home/runner/work/material-ui/material-ui/packages/mui-joy/tsconfig.build.json\n' +
    "../mui-base/build/TabsUnstyled/TabsUnstyled.d.ts(2,37): error TS2306: File '/home/runner/work/material-ui/material-ui/packages/mui-base/build/TabsUnstyled/TabsUnstyledProps.d.ts' is not a module.\n" +
    "../mui-base/build/TabsUnstyled/index.d.ts(6,51): error TS2306: File '/home/runner/work/material-ui/material-ui/packages/mui-base/build/TabsUnstyled/TabsUnstyledProps.d.ts' is not a module.\n" +
    'info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.\n',
  stderr: 'error Command failed with exit code 2.\n'
}
$ /home/runner/work/material-ui/material-ui/node_modules/.bin/tsc -b /home/runner/work/material-ui/material-ui/packages/mui-joy/tsconfig.build.json
../mui-base/build/TabsUnstyled/TabsUnstyled.d.ts(2,37): error TS2306: File '/home/runner/work/material-ui/material-ui/packages/mui-base/build/TabsUnstyled/TabsUnstyledProps.d.ts' is not a module.
../mui-base/build/TabsUnstyled/index.d.ts(6,51): error TS2306: File '/home/runner/work/material-ui/material-ui/packages/mui-base/build/TabsUnstyled/TabsUnstyledProps.d.ts' is not a module.

Multiple simultaneous calls to tsc -b for each package starts building declaration files for all of the referenced projects. Multiple projects have a reference to @mui/base and so several processes are trying to write the same declaration files at the same time and are interfering with each other (writes are not atomic). It can happen that typescript tries to read one of these declaration files in between two writes of chunks of the file. It would then get only part of a declaration file and it wouldn't be recognized as a correct declaration.

Different processes writing the same output is definitely not supported microsoft/TypeScript#42216. See also microsoft/TypeScript#38690

One possible solution, as presented in this PR, is to split the compilation and the type generation across the whole repository. Then we can rely on project references to build the types for the whole monorepo in a single tsc compilation step.

Another solution could be to not use the -b flag and instead rely on lerna to compile in the correct order. see #30201

Alternatively we can alter the prebuild script in the packages to not remove the build folder.

To Do:

@mui-pr-bot
Copy link

mui-pr-bot commented Dec 14, 2021

No bundle size changes

Generated by 🚫 dangerJS against 97832c6

@Janpot Janpot changed the title [build] Split up build process in compilation and types generation for the whole monorepo [build] Avoid types generation from interfering with itself Dec 14, 2021
@Janpot Janpot marked this pull request as ready for review December 15, 2021 09:29
@Janpot Janpot changed the title [build] Avoid types generation from interfering with itself [build] Avoid types generation from interferring with itself Jan 5, 2022
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label May 17, 2022
@michaldudak
Copy link
Member

@Janpot I haven't experienced the problem this PR (and #30201) try to solve for a long time. Should we close them?

@Janpot
Copy link
Member Author

Janpot commented Jun 27, 2023

Do we already know what causes these?

This error seems to come up in a flakey way, and it's happening in the same command

Error: Command failed: yarn tsc -b /home/runner/work/material-ui/material-ui/packages/mui-joy/tsconfig.build.json

Could this be another manifestation of the same race condition, or something else?

edit: I'm actually seeing the same error in vscode. It goes away when I use the workspace version of typescript for type checking instead of the vscode bundled one

@michaldudak
Copy link
Member

No, I haven't seen them. But these appear only with the newer TS versions, right? I have never encountered them locally.

@Janpot
Copy link
Member Author

Janpot commented Jun 27, 2023

Ok, verified test runs up until the point github actions don't provide logs anymore and can't find this error anymore. The race condition is probably still there as typescript still doesn't write files atomically, but it seems to not manifest itself anymore at this moment. I'd still recommend going forward with something like this PR as the current setup is not supported and thus may break at any time without semver guarantees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: out-of-date The pull request has merge conflicts and can't be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants