Skip to content

[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

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open

[WIP] Resolution sharing #61732

wants to merge 24 commits into from

Conversation

sheetalkamat
Copy link
Member

This is part of #55968 for creating buddy builds

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
@Copilot Copilot AI review requested due to automatic review settings May 19, 2025 20:37
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog May 19, 2025
@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label May 19, 2025
@sheetalkamat
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 19, 2025

Starting jobs; this comment will be updated as builds start and complete.

Command Status Results
pack this ✅ Started ✅ Results

@typescript-bot
Copy link
Collaborator

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.

Copy link

@Copilot Copilot AI left a 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 be verifyUnresolvedImports 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 extend CompilerHostSupportingResolutionCache may introduce ambiguity in the public API; consider merging or reorganizing these definitions to avoid confusion.
export interface LanguageServiceHost extends CompilerHostSupportingResolutionCache {

Comment on lines 1751 to 1754
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.
Copy link
Preview

Copilot AI May 19, 2025

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.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 19, 2025

Hey @sheetalkamat, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/165140/artifacts?artifactName=tgz&fileId=38F0D5FA6659216D35E44B9A03F5968D536E7D4D5A752C02365902CDC3C0AAC402&fileName=/typescript-5.9.0-insiders.20250519.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/[email protected]".;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

2 participants