[Build Performance] Updating types in a composite package causes a larger than necessary project tree rebuild

[berickson1]

Bug Report

🔎 Search Terms

typescript composite project performance type material ui

🕗 Version & Regression Information

Tested on @latest and @next. I do NOT believe this is a regression, but it is a performance issue for us.

Please keep and fill in the line that best applies:

• This is the behavior in every version I tried, and I reviewed the FAQ

⏯ Playground Link

Unfortunately, there's no good way to provide a playground, but see repo linked. This repo contains 22 composite projects:

• 20 identical button packages - which are simply usages of MUI Button (These cause each project's build to take an additional 500ms)
• core package - this is utilized by all the buttons
• zoo package - this imports a button (ultimately isn't really needed in this example)

💻 Code

1. Checkout Repo - https://github.com/berickson1/project-references-demo?organization=berickson1&organization=berickson1
2. yarn install
3. Run yarn watch (this runs tsc -b --watch in the root)
4. Open core/utilitites.ts and core/urelatedTypes.ts
5. Update the implementation of makeRandomName in core/utilities.ts. Observe built time (A)
6. Update the type UnrelatedType in core/unrelatedTypes.ts. Observe build time (B)

🙁 Actual behavior

Build A takes ~6s
Build B takes ~17s

In this case, updating an un-used and unrelated type in downstream packages causes a full re-build of the project. Unfortunately, some expensive types (described below in analysis section) cause this to be non-trivially expensive. Updating the implementation (step 5), rather than types, results in a quick rebuild since the downstream projects don't detect any changes in typings.

🙂 Expected behavior

Build types A and B should be similar - clocking in around 6s on my machine

My Analysis

I think this ultimately boils down to 2 related issues (please let me know if you'd rather this be tracked in separate reports)

1. The inclusion of MaterialUI button adds high overhead to type-checking (~0.5s per module it’s utilized in). Ideally this type would be cached across projects in some manner - especially since they’re using a templated tsconfig.json (Briefly discussed in: [Build Performance] Including yup adds 3 seconds to lib type-checking #46856 (comment) w/ @RyanCavanaugh)
2. Any update of exported types from a package causes re-checking for all downstream packages. Ideally the type-check should only be performed if the changed type is utilized by a downstream project.

Real-world use case

I recognize that the example repo provided is somewhat contrived, so I’ll share how we’re bumping into this.
Our repository has 100+ typescript project references that are wired together via projectReferences. Under the hood of our application we utilize GraphQL + Apollo, which has type-generation hooked up. This type-generation creates a collection of fragment/query/document types in a common package, which we then can import/access across the application (@myScope/gql-types). When a developer updates a GraphQL Query, the base @myScope/gql-types package gets updated, which triggers all packages that import any Queries to be re-compiled. We also take a dependency on MaterialUI in our packages, which increases the overhead of the incremental compile.

I would have initially hoped that skipLibCheck would have reduced the overhead of MUI type-checking, however I think the complex generics that are utilized in this (and other MUI components) still results in some heavy type inferencing.

Test Environment

2020 M1 MBP - 16GB Ram
Node 16.8.0
Typescript: 4.5.5

Potentially Related Issues/Discussions

Relates:
#47137 - This issue could potentially be addressed by something similar to d.ts tree-shaking proposed here
#43717

[sheetalkamat]

Your example show cases example where the change to unrelatedTypes.ts does not cause any errors.. But thats not always true.. Eg. consider this tsc-watch scenario that we test with its output here. Now assume that a.ts is in different project instead.. You can import only b.ts but change c.ts can cause new errors as well as changes in what needs to happen in your project. So its not update. Note that we do not have a way to detect unused types or something like that. Anything that exports and changes your declaration file is what is used to determine granularity of what needs update. (So file is a unit instead of each type)

[berickson1]

I believe what I talked about would in-fact find errors. I understand that there is likely design constraints as a limiting factor.

If you think about type consumption as a directed tree (from the root definition to the branches/leaves). A changes to any type node needs to propagate upwards away from the root and force a re-check of all usages at all nodes above. In the case of the unrelatedType, it's never consumed and has no references/usages, so type-checking could short early. If it was referenced, like in your a/b/c example, the change would invalidate unrelatedType (c.ts) and all it's usages (b.ts). That, in turn, would invalidate it's tree, causing a cascading type-cache invalidation in a.ts as well. Especially in codebases that have have high compile-time costs, this could be beneficial.

The above covers the the second portion of my analysis.

I still think that the first portion of my bug about material UI adding 500ms to the total build for every usage in a composite project is valid. The solution likely boils down to some level of caching as originally discussed in #46856

[typescript-bot]

This issue has been marked 'Working as Intended' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[berickson1]

@sheetalkamat FYI - don't know if you've read this before the bot decided this ticket was dead

[sheetalkamat]

But thats the thing.. our graph calculation is based on d.ts emit of a file and not type changes. So what you are saying isn't feasible that too across projects