Skip to content

Fixed a ghost error popping up after completions request caching a wrong contextual function type #58378

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 3 commits into
base: main
Choose a base branch
from

Conversation

Andarist
Copy link
Contributor

fixes #58351

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 30, 2024
@@ -1923,32 +1923,42 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function runWithoutResolvedSignatureCaching<T>(node: Node | undefined, fn: () => T): T {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This becomes more and more complicated and error-prone. I'm definitely not a fan of this. It builds on top of the existing solution... so at least it shouldn't be significantly worse now. It's just that perhaps there is a completely better way to do it than the existing mechanism. I think that alternatives could be explored separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about using infra introduced in #57421 but that PR is still in a draft. The problem space is also a little bit different - in this PR here we don't need to revert some recent work. In here, we need to temporarily evict some preexisting caches and restore them - we don't need to speculate per se. I assume that both are interested in the same lists of caches though.

const nodeLinks = getNodeLinks(node);
cachedResolvedSignatures.push([nodeLinks, nodeLinks.resolvedSignature, nodeLinks.flags] as const);
nodeLinks.resolvedSignature = undefined;
nodeLinks.flags = NodeCheckFlags.None;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key here is to reset NodeCheckFlags.ContextChecked so contextuallyCheckFunctionExpressionOrObjectLiteralMethod can correctly resolve (and cache) signature.resolvedReturnType multiple times (for requests with inference blocked and for requests without inference blocked). Otherwise, it has to be resolved later on... but without proper contextual information.

So I could just remove that bit and re-add it later if needed. I looked through all NodeCheckFlags and it might also be needed to do the same for NodeCheckFlags.TypeChecked. Although, I'm not 100% sure if that's really the case.

I just tried with a complete flags reset here and it does seem to work... but 🤷‍♂️ I'd probably be in favor of resetting solely what's really-really needed.

@typescript-bot typescript-bot added For Backlog Bug PRs that fix a backlog bug and removed For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jun 23, 2025
@github-project-automation github-project-automation bot moved this to Not started in PR Backlog Jun 23, 2025
@Andarist
Copy link
Contributor Author

@jakebailey could u build a playground for this one?

@jakebailey
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jun 23, 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

typescript-bot commented Jun 23, 2025

Hey @jakebailey, 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/165443/artifacts?artifactName=tgz&fileId=19A54E0F368D999E48289F2485543C4D299B83DBD5ECD3179955337BE892D2E802&fileName=/typescript-5.9.0-insiders.20250623.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/pr-build@5.9.0-pr-58378-3".;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
Status: Not started
Development

Successfully merging this pull request may close these issues.

Strange runtime caching bug involving as const and const T generic
3 participants