Pick correct compilerOptions when checking if we can share emitSignatures

[sheetalkamat]

OldCompilerOptions is set only if referencemaps are created vs not created. Module.None does not create reference map but other values do create it. Which is what determines if we can say copy semantic diagnostics etc..
But emit signatures are purely based on declaration path and their contents so they should only use the compilerOptions directly from old state.

Fixes #50902

[HolgerJeromin]

The fix works on my project (ported manually in JS code).
Thanks for the quick reaction.

[amcasey]

I think a comment would help to explain why this one spot doesn't use oldCompilerOptions. (To be honest, I couldn't figure it out from your PR description.) It's especially disconcerting that we might be dotting into oldState when useOldState is false, so please consider explaining that as well.

[sheetalkamat]

Please check if its more clear now.

[amcasey]

It sounds like maybe useOldState should be renamed? Maybe something like useOldStateFor...?

I'll attempt to paraphrase - please check my understanding:
We can only reuse emit signatures (i.e. .d.ts signatures) if the .d.ts file is unchanged, which will only be the case if the declarationDir and outDir options are unchanged.
We need to look in oldState.compilerOptions, rather than oldCompilerOptions (i.e. we need to disregard useOldState) because oldCompilerOptions will be undefined if the file references map indicates that the files to be emitted have changed, which isn't relevant for checking signature reuses.

Was I close?

[sheetalkamat]

We need to look in oldState.compilerOptions, rather than oldCompilerOptions (i.e. we need to disregard useOldState)

Since useOldState is determined by whether we create reference map to check for files to emit or not. So if say module option changes from Module.None to some other Module kind then oldCompilerOptions will be undefined but that shouldn't be taken into account when considering whether to reuse d.ts emit signature

[sheetalkamat]

Maybe something like useOldStateFor...?

not ideal since useOldStateFor since its everything - like semantic diagnostics, files to emit, changed files and so on except d.ts emit signature

[amcasey]

That's much clearer - thanks! For my own edification: every use case other than signature emit should still use oldCompilerOptions, correct?

[sheetalkamat]

Yes

[amcasey]

Thanks for the explanation!

[HolgerJeromin]

Can/should this fix be cherrypicked to the 4.8 branch?
Looks quite safe to me and v4.9 will be released in about 8 weeks.

[sheetalkamat]

@DanielRosenwasser and @RyanCavanaugh who can decide if it would be fit for 4.8 patch