Expose getDiagnosticsProducingTypeChecker to avoid duplicate type instantiations

[zzmp]

Search Terms

diagnosticsProducingTypeChecker
TypeChecker
getTypeChecker

Suggestion

Expose getDiagnosticsProducingTypeChecker.

Use Cases

A custom compiler adding additional checks must currently call:

// program: typescript.Program
const originalDiagnostics = program.get*Diagnostics(sourceFile);
const typeChecker: typescript.TypeChecker = program.getTypeChecker();
/* Use `typeChecker` to check the source file for additional diagnostics. */

Internally, get*Diagnostics methods use getDiagnosticsProducingTypeChecker, so it is already "warmed" - types are already instantiated. The current implementation requires that a compiler pay the cost to instantiate types twice: once with a diagnosticsProducingTypeChecker, and once with a noDiagnosticsTypeChecker.

This would eliminate the redundant warm-up cost for instantiating types in the TypeChecker. This would be especially effective in code bases with deeply nested type definitions which may take noticeably longer to instantiate.

I do not fully understand the difference between the two TypeCheckers, so I cannot honestly comment on the drawbacks to exposing/using a diagnosticsProducingTypeChecker for custom checks (can someone please explain this, or point me towards documentation?). If it is unfeasible, perhaps there is a way to warm both checkers on the same run, to reduce the repeated type instantiations.

Examples

See Use Cases.

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript / JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. new expression-level syntax)

[evmar]

This comes up in http://tsetse.info/. cc @alexeagle

[sheetalkamat]

Diagnostic producing checker is reserved for reporting errors since it needs to ensure that types were not materialized without checking. And hence usage of this checker for anything else is not advised.

[zzmp]

@sheetalkamat can you go into more detail?

Is there any reason not to use this checker after semantic checking has been run?
Can you give a case where using this checker for custom checks would cause adverse effects?
Are the details of the diagnostic checker documented anywhere? Is there any reason that the initialization of the checker can't also initialize the non-diagnostic-producing checker?

[zzmp]

@sheetalkamat @weswigham (or anyone really), what would happen if one were to reuse the diagnostic producing checker for other checks? In my own testing I haven't seen any effects (outside of 20-30% performance gains from not having to instantiate a new checker), so I'm still very interested in this.

It seems like the checker does checks when it instantiates types but, if you are not instantiating any new types (i.e. you've already checked your whole program) then it is fine to continue to use the checker to re-check those types. Am I mistaken? I'd really like to understand why two checkers exist.

[weswigham]

AFAIK the only real reason there's a distinction is that. supposedly, if you're only using a noDiagnosticsProducingTypeChecker, you saved on doing some work that the diagnostics producing one would otherwise have done (in all fairness, I think at this point most of the things that we check produceDiagnostics for in checker.ts are not high cost in the grand scheme of things). TBQH, I think getTypeChecker should just return the diagnostics producing one if it has already been constructed to fetch diagnostics from. The issue there, though, becomes that then you don't know which checker you may have, and types from different checkers aren't comparable, which is a big problem - which leads to the constraint that the public function to produce a checker must always return the same checker, regardless of when it's called.

We've talked about this internally before, and that's mostly an issue in how the checker is currently structured - there are really two things the checker needs to do; construct and materialize types, and issue diagnostics when those types have errors. Today, the two are interspersed within the same codepaths, which makes it quite difficult to divorce one from the other; in an ideal setting, they'd probably be different walks of the type hierarchy.

Anyways - I think we should probably revisit this at some point; because repeating the same work (like checking the .lib.d.ts) does feel bad.

[zzmp]

Is there a compelling reason not to expose it so that use cases that already produce the diagnosticsProducingTypeChecker can just reuse it instead of instantiating a new one? Is there some scenario where users can shoot themselves in the foot by misunderstanding this?

[weswigham]

I've opened #28584 with a quick edit of what seems like a decent compromise - if it exists, using the same checker that was made for diagnostics, otherwise making the work-saving one.

[filipesilva]

Just wanted to mention that at Angular we were investigating the impact some extra diagnostics we run had on build times (angular/angular#27426).

We arrived at the conclusion that most of the time spent in the extra diagnostics was solely due to calling the type checker. Checks that didn't use the type checker maxed out at 102ms while checks that used it maxed at 2012ms. So up to 20x longer.

Moving some stuff around in those rules (bazelbuild/rules_typescript#359) gave us a 18% smaller build time for the compilation unit we were testing.

In our case we were using 5 extra checks, 3 of which used the type checker. I can imagine more involved setups having their build time further impacted.