-
-
Notifications
You must be signed in to change notification settings - Fork 536
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
Implement getProjectVersion #963
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, feel free to ignore the comments. I'm not sure how useful the debug statements are if the conditions are as mentioned in the comment - maybe better as an invariant
check that throws?
if (!isFileInCache) { | ||
rootFileNames.push(fileName) | ||
// Modifying rootFileNames means a project change | ||
shouldIncrementProjectVersion = true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: You could just do projectVersion++
here - it's essentially the same, an extra +1 every now and then isn't a big deal.
src/index.ts
Outdated
@@ -469,6 +470,7 @@ export function create (rawOptions: CreateOptions = {}): Register { | |||
|
|||
// Create the compiler host for type checking. | |||
const serviceHost: _ts.LanguageServiceHost = { | |||
getProjectVersion: () => `${ projectVersion }`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Personally I'd just do String(projectVersion)
since you just want to type cast.
Yeah, I wasn't sure about that. I really wanted to avoid ts-node crashing if new versions of the TypeScript compiler misbehaved. I figured better for us to remain functional with degraded performance and let the debug logs point out why. |
Due to microsoft/TypeScript#36748, TypeScript's language service always thinks the
Program
is out-of-date and rebuilds it 3 times for every.ts
file that we load: once each forgetEmitOutput
,getSemanticDiagnostics
, andgetSyntacticDiagnostics
.We can workaround this bug by implementing
getProjectVersion
. IfgetProjectVersion
stays the same, TypeScript will reuse the existingProgram
. It'll still need to be rebuilt every time we modify the rootFiles array, but that's only once instead of 3 times.I also added debug log statements that double-check when the
Program
instance is rebuilt. This should be helpful if we ask for TS_NODE_DEBUG logs from users.