Skip to content

Conversation

@amcasey
Copy link
Member

@amcasey amcasey commented Feb 19, 2022

Only the last commit is interesting - the others are in #47938.

At this point, isDefinition is wrong in ways that probably don't matter. The baselines show the different per-project FAR invocations and the product diff indicates the maximum simplification we can achieve (adding
code to fix up isDefinition can only make it worse).

I'm generally okay with the test change (seeing fewer results when a file is specified) both in principal, since not loading projects seems preferable when scoping to the active document, and in practice, since AFAIK only old version of VS Code pass a file with no project.
This change had two goals:

1. Make the code easier to understand, primarily by simplifying the callback structure and minimizing side-effects
2. Improve performance by reducing repeated work, both FAR searches of individual projects and default tsconfig searches

Most of the baseline changes just reflect the de-duping of tsconfig searches, a few show fewer and occasionally different FAR invocations, and a couple show response changes.  I'm quite confident that the new FAR calls are better and moderately confident that the new isDefinition values are better (not to mention moderately skeptical that anyone will ever hit a case where the difference matters).
At this point, `isDefinition` is wrong in ways that probably don't matter.  The baselines show the different per-project FAR invocations and the product diff indicates the maximum simplification we can achieve (adding code to fix up `isDefinition` can only make it worse).
@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Feb 19, 2022
@amcasey amcasey closed this Mar 3, 2022
@amcasey amcasey deleted the FarIsDefinition branch March 3, 2022 00:05
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 22, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants