Skip to content

Request completion entry details from auto-import entries. #95

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

Merged
merged 1 commit into from
Feb 7, 2023

Conversation

DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Feb 7, 2023

Also doubles the auto-import request rate from 1% to 2%.

if (entry.hasAction && entry.source !== undefined && "data" in entry) {
const { name, source, data } = entry;

// 'completionEntryDetails' can take multiple entries; however, it's not useful for diagnostics
Copy link
Member Author

Choose a reason for hiding this comment

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

Curious to hear if others think this is bad - this is pretty likely to be slower, but I like to look at the last command and know exactly what went wrong.

I could make requests on 5 at a time in the future.

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK nobody uses the multiple-details-at-a-time command. IIRC it’s not optimized whatsoever; it just loops at the super top level and doesn’t cache anything.

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Details requests are usually pretty fast; I doubt this will slow the whole process down that much.

@DanielRosenwasser DanielRosenwasser merged commit 7ee1590 into main Feb 7, 2023
@DanielRosenwasser DanielRosenwasser deleted the autoImportCompletionEntryDetails branch February 7, 2023 19:27
@DanielRosenwasser
Copy link
Member Author

microsoft/TypeScript#52660 is the first run with this enabled... and it is brutal!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants