-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Skip typechecking file when generating declaraiton to get d.ts signature for incremental build #58592
Conversation
sheetalkamat
commented
May 20, 2024
•
edited
Loading
edited
- Skips typechecking when emitting d.ts for determining if signature of the file has changed.
- Sets cancellation token for emitting files to ensure that it is cancellable esp since files may not be type checked
tests/baselines/reference/tsc/cancellationToken/when-emitting-buildInfo.js
Outdated
Show resolved
Hide resolved
tests/baselines/reference/tsc/cancellationToken/when-using-state.js
Outdated
Show resolved
Hide resolved
tests/baselines/reference/tsbuildWatch/demo/updates-with-bad-reference.js
Outdated
Show resolved
Hide resolved
...sc/incremental/change-to-modifier-of-class-expression-field-with-declaration-emit-enabled.js
Outdated
Show resolved
Hide resolved
@weswigham i tried passing noCheck equivalent when running emit for dts emit and i see few tests failing. i have commented on each but if you can take a look and see what we can do for those scenarios that would be great. Thanks |
…n resolve types etc without type checking
@typescript-bot perf test this faster |
516c734
to
5cc1ea4
Compare
@jakebailey Here they are:
tscComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
Is this the winning PR or is #58593 still something to consider? Just checking before I review... |
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.
Is this the winning PR or is #58593 still something to consider? Just checking before I review...
They don't appear mutually exclusive to my eye. This one skips the diagnostic pass when calculating signatures. The other skips it when a //@ts-nocheck
comment is in a file.
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.
Oh, one caveat though - #58364 isn't in yet, so JS emit is unsafe when you skip diagnostics like this. Is an emit resolver with forceDtsEmit
set guaranteed to be only emitting .d.ts
files?
I don't think that it is guaranteed; I almost made the playground always set forceDtsEmit because it always needs to show it (playground used to crash on dts errors), though I think I actually made it conditional in the current version. |
Yes it is guaranteed to be dts only emit as "forceDtsEmit" is internal on program.emit and used only by builder |
Isn't it public via |
LS.. compile on save.. forgot about that. :( i will upate the condition for now to check if its emitOnlyDts and forceDtsEmit to be true which we can later update in #58364 to remove emitOnlyDts part. |
…it to be done without type checking