-
Notifications
You must be signed in to change notification settings - Fork 12.8k
[WIP] Resolution sharing #61732
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
base: main
Are you sure you want to change the base?
[WIP] Resolution sharing #61732
Conversation
First unresolved import gets the typing file into cache and sets it as root, This results in scheduling new typing request with that unresolved import missing The result will come back with typing file that is omitted, so root file will change but file will still be part of program so its not really removed from program
Also cache the inferred type acquition for inferred project
…ypings cache This change finally makes all tests pass incremental tests for matching resolutions and program structuture
…eed to update last cached unresolved imports just because we have new program
…s in the program This shows cache is holding onto resolutions that are no longer needed by program because either those modules arent present in file or is determined to be ambient resolution
…ns are reused/are resolved to ambient module names
…ay instead of defering external module reoslutions to watch all failed lookup locations
- All the package json watched are ref counted if watched - Not watched package json locations are released - First of this kind are when resolution from global cache fails, we dont watch those locations so not safe to have them cached - If we are looking for a file and the file is not found, the package json locations looked up are not watched
…root directory for watching resolutions
@typescript-bot pack this |
Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page. Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up. |
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.
Pull Request Overview
This PR adds shared resolution cache support and refines module resolution behaviors, introduces signal-based changes to the typing installer and resolution cache, and expands test coverage for various resolution and type acquisition scenarios.
- Ensure resolution cache invalidation respects new global‐cache pass logic
- Wire through new callbacks (
onReused*
,onSourceFileNotCreated
) in language service hosts and watch APIs - Expand automated tests for type acquisition changes and resolution cache lifetime across tsserver and tsc-watch
Reviewed Changes
Copilot reviewed 969 out of 969 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
src/typingsInstallerCore/typingsInstaller.ts | Optimize change detection by comparing set sizes |
src/testRunner/unittests/tsserver/typingsInstaller.ts | Removed outdated assert.isFalse checks around invalidated imports |
src/testRunner/unittests/tsserver/typeAquisition.ts | Added comprehensive typeAquisition change tests |
src/services/types.ts | Added CompilerHostSupportingResolutionCache extension to LanguageServiceHost |
src/server/session.ts | Simplified entrypoint check by dropping null guard |
Comments suppressed due to low confidence (4)
src/services/editorServices.ts:1432
- [nitpick] The method name
verifyUnresovedImports
is misspelled; it should beverifyUnresolvedImports
for clarity and consistency.
verifyUnresovedImports: (project: Project) => void = noop;
src/testRunner/unittests/tsserver/typingsInstaller.ts:2412
- These assertions were removed, potentially reducing test coverage for invalidation behavior; consider adding a focused test case to verify that non-relative unresolved imports are invalidated appropriately.
assert.isFalse(proj.resolutionCache.isFileWithInvalidatedNonRelativeUnresolvedImports(app.path as ts.Path));
src/testRunner/unittests/tsserver/typeAquisition.ts:64
- [nitpick] The term "typeAquisition" is misspelled; it should be "typeAcquisition" to match established naming conventions.
describe("unittests:: tsserver:: typeAquisition:: changes", () => {
src/services/types.ts:441
- [nitpick] Redeclaring
LanguageServiceHost
to extendCompilerHostSupportingResolutionCache
may introduce ambiguity in the public API; consider merging or reorganizing these definitions to avoid confusion.
export interface LanguageServiceHost extends CompilerHostSupportingResolutionCache {
const path = project.toPath(fileName); | ||
if (entrypoints && some(entrypoints, e => project.toPath(e) === path)) { | ||
if (some(entrypoints, e => project.toPath(e) === path)) { | ||
// This file was the main entrypoint of a package. Try to resolve that same package name with | ||
// the auxiliary project that only resolves to implementation files. |
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.
The null/undefined check for entrypoints
was removed, so calling some
on undefined
can throw; reintroduce a guard (e.g. entrypoints && some(...)
) to avoid runtime errors.
Copilot uses AI. Check for mistakes.
Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your
and then running There is also a playground for this build and an npm module you can use via |
This is part of #55968 for creating buddy builds