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

TypeScript compiler stucks with --incremental flag #31258

Closed
ms-qinyang opened this issue May 5, 2019 · 6 comments
Closed

TypeScript compiler stucks with --incremental flag #31258

ms-qinyang opened this issue May 5, 2019 · 6 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@ms-qinyang
Copy link

TypeScript Version: 3.4.5

Search Terms:
stuck, slow, incremental

Code

https://github.com/ms-qinyang/typescript-stuck-incremental

run npm run buildIncremental

PS D:\stuck-with-incremental> Measure-Command { npm run build | Out-Host }

> stuck-with-incremental@1.0.0 build D:\stuck-with-incremental
> tsc -p .



Days              : 0
Hours             : 0
Minutes           : 0
Seconds           : 1
Milliseconds      : 890
Ticks             : 18903973
TotalDays         : 2.18795983796296E-05
TotalHours        : 0.000525110361111111
TotalMinutes      : 0.0315066216666667
TotalSeconds      : 1.8903973
TotalMilliseconds : 1890.3973



PS D:\stuck-with-incremental> Measure-Command { npm run buildIncremental | Out-Host }

> stuck-with-incremental@1.0.0 buildIncremental D:\stuck-with-incremental
> tsc -p ./tsconfig-incremental.json



Days              : 0
Hours             : 0
Minutes           : 1
Seconds           : 3
Milliseconds      : 989
Ticks             : 639891999
TotalDays         : 0.000740615739583333
TotalHours        : 0.01777477775
TotalMinutes      : 1.066486665
TotalSeconds      : 63.9891999
TotalMilliseconds : 63989.1999

Expected behavior:

Compiler should be finished in a short time

Actual behavior:

Compiler takes more than 1 minute to finish. Since the real project has many files, the compiler won't be finished in an acceptable time. This breaks typecheck in create-react-app after 2.1.8

Related Issues:

@weswigham weswigham added Bug A bug in TypeScript Domain: --incremental The issue relates to incremental compilation Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: Performance Reports of unusually slow behavior labels May 8, 2019
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 3.6.0 milestone Jun 12, 2019
@sheetalkamat
Copy link
Member

This seems to be stuck in declaration emit.

@weswigham
Copy link
Member

weswigham commented Aug 14, 2019

One of the declaration files we produce is 209163 lines long. (specifically the output for createModel2) 150,000 of those lines are spent repeating the anonymous type

type repeated = {
    type: TConfigs[number]["type"];
    str1: string;
    str2: string;
}

and the rest are spent nesting a bunch of redux-advanced types, specifically, specifying the type arguments for SelectorContext. I would say that normally what we'd like to do here is preserve aliases better (mapped types, especially), but with all the fluent mapping and building going on in redux-advanced, I'd be surprised if there's anything approaching a short way to refer to any of this - all throughout the createModel2 definition, there's truncation in the quickinfo, just because all of the types for everything are so complex - the very first chained method call returns a ModelBuilder<TDependencies, TProps, TState extends object ? TState & T : T, TSelectors, TReducers, TEffects, TEpics>; which has it's inferred argument type inserted, and that result is truncated in quickinfo because it's just too much. Then that's flowed into every subsequent call which layers on more and more stuff. There is simply not a short name for this stuff - it's a grand amalgam, a great composition, of no fewer than 10 inferred types which are combined and composed in numerous ways.

Our canonical response in cases like these, thus far, is to recommend adding a type annotation to the offending declaration, so we can avoid the generation process entirely.

Now, while I said that I don't think all of the complexity in the printback can be avoided, I think #30979 or something like it (specifically the let ... in style variant) could help quite a bit - then we'd be able to hoist all these common anonymous types to the outermost area wherein they were valid and avoid repeating them so much.

@weswigham
Copy link
Member

TL;DR: There's no simple "fix" here because nothing is going "wrong" - our declaration emit just just less expressive than the input code, and it shows in the complexity of the output. At least the work around is pretty easy to do: add a type annotation on your function returns (which, given that a builder pattern is in use, probably isn't what anyone wants to hear, as it kills an advantage to the strongly typed builder, but it's what we have right now).

@RyanCavanaugh RyanCavanaugh added Design Limitation Constraints of the existing architecture prevent this from being fixed and removed Bug A bug in TypeScript Domain: --incremental The issue relates to incremental compilation Domain: Declaration Emit The issue relates to the emission of d.ts files Domain: Performance Reports of unusually slow behavior labels Aug 14, 2019
@RyanCavanaugh RyanCavanaugh removed this from the TypeScript 3.7.0 milestone Aug 14, 2019
@AnyhowStep
Copy link
Contributor

AnyhowStep commented Aug 14, 2019

Our canonical response in cases like these, thus far, is to recommend adding a type annotation to the offending declaration, so we can avoid the generation process entirely.

This. I've been burned by it too many times that I've basically requested a lint rule be written for it,
typescript-eslint/typescript-eslint#730

When it comes to generic functions/methods, always err on the side of caution and explicitly annotate the return type. Even if the expected emit is "obvious" (to a human).

If you're composing them, this rule goes triple.

You'll save yourself a lot of declaration emit headaches.


Without a lint rule, you can just... Forget to annotate. I'm pretty rigorous about it, usually. But the one time I forgot, my build time became insane.


One time I forgot to explicitly annotate, it ended up emitting weird stuff,
#32097


I'd say I have quite a lot of experience with generic functions/methods, composing/chaining them, and fluent/functional APIs.

It's not unusual for me to have 300 LoC on one file and only 36 lines are executable JS. The other 264 lines are mostly type declarations (interfaces, classes, type aliases) and comments. More often than not, the comments are maybe 50 LoC or less


This is one of the more "tame" files I've written recently,
https://github.com/AnyhowStep/tsql/blob/1ce84eecf98a0016d985d338de36de6eb74fe7f6/src/from-clause/util/operation/where-eq-outer-query-primary-key.ts

Mostly comments, a bunch of type aliases. And, finally, executable code at the very bottom.

That "util" function is later used by a class that implements a builder pattern in a different project,
https://github.com/AnyhowStep/tsql-mysql-5.7/blob/2ccc0865323135fe48587a3b290fbba1d2ba61c9/src/query/query-impl.ts#L195


One of the declaration files we produce is 209163 lines long.

200k lines is better than being OOM during emit. At least the user can see output and go "that's strange" without having to attach a debugger!

@ms-qinyang
Copy link
Author

Thanks for the investigation. However, the reason we use this library is that we don't want to write unnecessary type definitions explicitly. Since it works well without incremental flag, we will override CRA configuration to avoid this issue.

@weswigham
Copy link
Member

weswigham commented Aug 14, 2019

It's probably also worth noting that your config had both incremental an noEmit on, which just... Does extra work for no gain. That it's not an error is an oversight on our part right now. You at least need to emit declaration files and the incremental bundle info.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

6 participants