Uh oh!
There was an error while loading. Please reload this page.
Show implied options in --showConfig - #56701
Conversation
Andrew Branch (andrewbranch)
commented
Dec 6, 2023
TypeScript Bot (@typescript-bot) perf test this |
Heya Andrew Branch (@andrewbranch), I've started to run the regular perf test suite on this PR at 1032991. You can monitor the build here. Update: The results are in! |
| /** @internal */ | ||
| export const getEmitModuleDetectionKind = computedOptions.moduleDetection.computeValue; | ||
| /** @internal */ | ||
| export const getIsolatedModules = computedOptions.isolatedModules.computeValue; |
There was a problem hiding this comment.
One advantage is that these functions now automatically say what they depend on in their parameter type. The signature of this one, for example, is (compilerOptions: Pick<CompilerOptions, "isolatedModules" | "verbatimModuleSyntax">) => boolean
TypeScript Bot (typescript-bot)
commented
Dec 7, 2023
Andrew Branch (@andrewbranch) Here they are:CompilerComparison Report - baseline..pr
System info unknown Hosts
Scenarios
tsserverComparison Report - baseline..pr
System info unknown Hosts
Scenarios
StartupComparison Report - baseline..pr
System info unknown Hosts
Scenarios
Developer Information: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
This seems reasonable to me.
I know the perf doesn't seem to be affected, but I do wonder if we should lift all of these calls in the checker upward so they're all precalculated and local, rather than continually calculating them. e.g. we call getEmitModuleResolutionKind and getESModuleInterop a looot. As evidenced by this change, they all strictly depend on compilerOptions, so could be known immediately.
Uh oh!
There was an error while loading. Please reload this page.
| moduleResolution: computedCompilerOption("moduleResolution", ["module", "target"], (compilerOptions): ModuleResolutionKind => { | ||
| let moduleResolution = compilerOptions.moduleResolution; | ||
| if (moduleResolution === undefined) { | ||
| export const computedOptions = createComputedCompilerOptions({ |
There was a problem hiding this comment.
Thanks Mateusz Burzyński (@Andarist) for helping with this tricky inference 🌟
This only works in recent nightlies, hence the “LKG” bump (cc Jake Bailey (@jakebailey))
There was a problem hiding this comment.
I don't mind bumping the TS version, though I am wondering which PR was needed to make this pattern work (and of course if there's any change at all that we could avoid it 😅).
But yeah, this pattern is nicer than the previous IMO.
There was a problem hiding this comment.
It’s the recent improvement from #55811 . It was merged in just last week :)
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Sheetal Nandi (sheetalkamat)
left a comment
There was a problem hiding this comment.
Looks good apart from minor nits
Fixes my minor annoyance from earlier today
--showConfigon this tsconfig:{ "compilerOptions": { "module": "NodeNext" } }Before:
{ "compilerOptions": { "module": "nodenext" }, "files": [ "./main.ts", ] }After:
{ "compilerOptions": { "module": "nodenext", "target": "esnext", "moduleResolution": "nodenext", "moduleDetection": "force", "esModuleInterop": true, "allowSyntheticDefaultImports": true, "useDefineForClassFields": true }, "files": [ "./main.ts" ] }