[Build Performance] Including yup adds 3 seconds to lib type-checking

[berickson1]

Bug Report

In our (private) typescript project that utilizes composite projects, we've been struggling with increasing compile times as more projects have been added. I took a stab at investigating this based on the Performance Tracing Docs. Ultimately I found that consuming yup adds ~3s to each package build (on an M1 MBP). Given we have 100+ composite projects in the repo (not all of which are impacted, since not all import yup), this starts to stack up as a non-trivial increase to build times.

This can be mitigated by setting skipLibCheck to true, but ideally this would be false for complete validation of downstream types.

🔎 Search Terms

performance yup lib type checking

🕗 Version & Regression Information

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

⏯ Playground Link

Playground link with relevant code
The above playground link isn't overly useful given this is a performance bug, I've also prepared a minimal repro. https://github.com/berickson1/Playground/tree/master/type-checking-perf

💻 Code

import yup from 'yup'

export function testFunction(): void {
    const yupString = yup.string()
    console.log(yupString);
}

🙁 Actual behavior

Project compile time went from sub-second to over 3s on an M1 MBP to a single project. When using composite projects, this has a larger impact for every package that includes yup.

Trace ZIP:
trace.json.zip

🙂 Expected behavior

Project compile sees a trivial increase when including yup. This could either be managed by 1) improving peformance of type-checking or 2) leveraging a shared in-memory cache for node_module lib types across composite builds

[RyanCavanaugh]

I'm not quite sure how to turn this into something actionable. The types are just legitimately extremely complex. Here's a snippet from date.d.ts

export interface RequiredDateSchema<TType extends Maybe<Date>, TContext extends AnyObject = AnyObject> extends DateSchema<TType, TContext, NonNullable<TType>> {
    // Cheap half
    defined(msg?: MixedLocale['defined']): DefinedDateSchema<TType, TContext>;
    required(msg?: MixedLocale['required']): RequiredDateSchema<TType, TContext>;
    optional(): DateSchema<TType, TContext>;
    notRequired(): DateSchema<TType, TContext>;

    // Expensive half
    default<D extends Maybe<TType>>(def: Thunk<D>): If<D, RequiredDateSchema<TType | undefined, TContext>, RequiredDateSchema<Defined<TType>, TContext>>;
    nullable(isNullable?: true): RequiredDateSchema<TType | null, TContext>;
    nullable(isNullable: false): RequiredDateSchema<Exclude<TType, null>, TContext>;
}

Commenting out the "expensive half" saves about 0.4s (!!), but it's easy to see why -- validating that all of these instantiations are legal is a huge amount of work. We might be able to make this a little faster, but we're not going to be able to check this fast enough that it won't have a measurable impact on your overall build perf. Even a 10x improvement from 3s to 0.3s is still going to add up to a lot over multiple builds.

It's not possible for us to reuse validation results between builds because each project in a composite build might have different settings or pull in different types (in fact, the latter is almost certainly the case in most builds), both of which can affect whether or not an arbitrary lib typechecks successfully or not.

If you're pulling in lib files someone else published to npm, skipLibCheck is just a very very safe thing to turn on and we recommend it universally for exactly this reason.

[berickson1]

I've personally seen a handful of type breaks of downstream types as package upgrades land, so had left this disabled for safety.

We also have some complex recursive types in our repo - so improvement here could potentially have downstream benefits to others, but it's hard to directly enumerate. I'm starting to do some performance profiling of our project to identify and improve hotspots.
That being said, I'll move forward with setting "skipLibCheck": true to realize some gains here on our builds.

[RyanCavanaugh]

Ideas that @amcasey and I had after spitballing on this for a while:

• Add a build-mode switch to only check .d.ts files once per build operation, even if this isn't necessarily correct (name it --skipLibCheckYolo or whatever)
• Somehow group up projects based on some definition of "environment" (settings, relevant global augmentations, etc) and pass multiple projects through the checker at once assuming their size isn't too big. We could do this grouping optimistically based on the tsbuildinfo of the prior build and then bail out if one of the assumptions was invalidated. There are a lot of projects with "many too-small projects" setup that would benefit greatly from this, but would be very difficult to get right in a way that amortizes well since there's a lot of information that would go into computing the "environment".
  • Sub-idea I had while typing this: We could make projects explicitly define a "cohort" property that says "I avow that it's safe to group me with other projects in the same cohort". That puts the correctness impetus on the user (a trade-off to be sure) but might be statically verifiable under a separate pass (tsc --build --check-cohorts ?).

[berickson1]

I think either of those would help our scenario - our setup basically templated tsconfig files (see below) so there shouldn't be any additional types included. Of the above, I do like the idea of grouped cohorts - that way it's still very explicit that behaviour is tied together (perhaps a simple checker could be put in place to validate that there's consistency in tsconfig.json files in the same cohort)

{
  "extends": "../../../tsconfig.base.json",
  "compilerOptions": { "rootDir": "./src", "outDir": "./dist" },
  "include": ["src/**/*", "src/**/*.json"],
  "references": [
/* referencePathsHere */
  ]
}

[ahejlsberg]

FWIW, in 4.6 we're doing a lot better on yup because of #46599. The check time for the yup definitely typed package is 5.0s with 4.5, but only 1.5s with 4.6. That's more than a 3x reduction.

[berickson1]

FWIW, in 4.6 we're doing a lot better on yup because of #46599. The check time for the yup definitely typed package is 5.0s with 4.5, but only 1.5s with 4.6. That's more than a 3x reduction.

I can confirm that setting skipLibCheck: false takes my measured sample (~3s) of yup processing time down to ~600ms for the 3 highlighted expensive .d.ts files utilizing "typescript": "4.6.0-dev.20211120",.