Don't use "composite": true in tsc (until it supports cycles)

[nicolo-ribaudo]

Q	A
Fixed Issues?
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

Currently TSC doesn't support composite projects with circular references. This is a problem for us, since (especially when considering peer dependencies) we have many of them.

We could choose to ignore peerDependencies when computing project references, but that ends up with import x from "@babel/core" being typed as any in packages with a peerDep on @babel/core. If we just stop using "composite": true, we don't have this problem because TS will load everything at the same time.

This has some overhead when running make tscheck, editing some files in a single package, and then running make tscheck again: TS will have to check the whole monorepo rather than just the compiled files.
However, almost any editor now has a native TS integration, so make tscheck is something we only run once before committing.

cc @zxbodya @JLHwung

[babel-bot]

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/45785/

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c7c0b2b:

Sandbox	Source
babel-repl-custom-plugin	Configuration
babel-plugin-multi-config	Configuration

[zxbodya]

I do not think there is a need to include peerDependencies in project references. In most cases - it does not need to be included. And everything currently in the codebase works just fine if to include only dependencies referenced in source code and dependencies in package.json (I already did that in #11578). For @babel/core, it is also ok, in most cases - for usages currently in the codebase, it does not cause circular dependency.

However, there are indeed cases that currently have a good reason for circular dependency, recent example with helper which is not yet merged - #13122 (helper using NodePath from @babel/traverse used by @babel/traverse itself)

Not sure what is the best way to solve cases like this, my gut feeling - there should be a good way to avoid circular dependency, either making that helper a part of @babel/traverse or maybe better - splitting part of @babel/traverse into a separate package.

Generally - I would prefer not to have circular dependencies, I think this would enforce better separation of concerns. And having composite build in ts is a good bonus. But that might be a big change, and currently is not very clear how big.

---

About the change itself - added some comments, but should be fine.

This will likely make type checks a bit slower, but I guess it is unlikely to be very noticeable(as I can understand, ts server is good about updating its state incrementally).

I think, it is better to come back to question about having circular dependencies after converting everything to TypeScript and improving type coverage, which will provide better understanding about the options and effort required to "fix" all circular dependencies. Also, maybe microsoft/TypeScript#33685 would be an option to solve this sometime in the future.

[zxbodya]

What about eslint and codemods? - I guess eventually it should also be migrated to typescript

[zxbodya]

Why there is a warning about it? Can it cause issue when publishing d.ts files?

[nicolo-ribaudo]

It doesn't cause issues because types are hoisted. It's only a problem when two modules depend on each other and they export/import non-hoistable declarations:

// a.js
import { b } from "b";
export let a = b;

import { a } from "a";
export let b = a;

[nicolo-ribaudo]

However, there are indeed cases that currently have a good reason for circular dependency, recent example with helper which is not yet merged - #13122 (helper using NodePath from @babel/traverse used by @babel/traverse itself)

Yes, that was exactly the PR that made me prepare this one. Also, I think that for now we don't have many circular dependencies similar to that one, but as soon as we start improving our internal type annotations they will happen more often.

At least for now, I would like to avoid refactoring our code structure to accommodate TS: we should try to make TS work with what we have.

Also, maybe microsoft/TypeScript#33685 would be an option to solve this sometime in the future.

Thanks, I was looking for that issue but I couldn't find it 😅 That's why the original title of this PR was "Don't use "composite": true in tsconfig.json until it supports cycles": I'd like to go back to project references as soon as they do. I'll leave a comment in that issue.