-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix type import node circularity errors in eager diagnostic mode, deprecations #52861
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
Conversation
@typescript-bot test this |
Heya @jakebailey, I've started to run the extended test suite on this PR at 59ce29c. You can monitor the build here. |
Heya @jakebailey, I've started to run the diff-based user code test suite on this PR at 59ce29c. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the diff-based top-repos suite (tsserver) on this PR at 59ce29c. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the parallelized Definitely Typed test suite on this PR at 59ce29c. You can monitor the build here. |
Heya @jakebailey, I've started to run the diff-based top-repos suite on this PR at 59ce29c. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the diff-based user code test suite (tsserver) on this PR at 59ce29c. You can monitor the build here. Update: The results are in! |
Heya @jakebailey, I've started to run the perf test suite on this PR at 59ce29c. You can monitor the build here. Update: The results are in! |
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
1 similar comment
@jakebailey Here are the results of running the user test suite comparing Everything looks good! |
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
1 similar comment
@jakebailey Here are the results of running the top-repos suite comparing Everything looks good! |
@jakebailey Here they are:
CompilerComparison Report - main..52861
System
Hosts
Scenarios
TSServerComparison Report - main..52861
System
Hosts
Scenarios
StartupComparison Report - main..52861
System
Hosts
Scenarios
Developer Information: |
Fixes #52570
Even though the name implies otherwise, a callback passed
addLazyDiagnostic
may be executed immediately if the checker is in eager mode.The new code added in #51766 ended up causing a circularity error because in eager mode it was executed before
links.resolvedType
was set (which happens outsideresolveImportSymbolType
).Fix this by effectively reverting #51766 and instead opting to reuse the existing code for type references. Reusing the existing code for this (instead of partially copying it) also has the benefit of fixing deprecated diagnostics which were broken for type import nodes (but I haven't been able to find anyone who reported such a thing yet).