Skip to content
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

Hitting enter on suggestion doesn't import module #73311

Closed
jordyvandomselaar opened this issue May 5, 2019 · 17 comments
Closed

Hitting enter on suggestion doesn't import module #73311

jordyvandomselaar opened this issue May 5, 2019 · 17 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) javascript JavaScript support issues suggest IntelliSense, Auto Complete

Comments

@jordyvandomselaar
Copy link

jordyvandomselaar commented May 5, 2019

Issue Type: Bug

For this example I'll be using React.

I have the following components;

import styled from "styled-components";

type Justify = "space-around" | "space-between";

export const FlexBox = styled.div<{
  column?: boolean;
  justify?: Justify;
}>`
  display: flex;
  flex-direction: ${props => (props.column ? "column" : "row")};
  ${props => !!props.justify && `justify-content: ${props.justify}`}
`;

export const Flex = styled.div<{ flex?: number }>`
  flex: ${props => props.flex || 1};
`;

When I try to use them inside another component I do get an auto completion with the correct name whilst typing FlexBox but when I hit enter it doesn't import it (it does complete it), clicking on the suggestion with my mouse does work.

Here's a video describing the problem;

https://monosnap.com/file/dqiy5IbunVfW9ExUZqFwusQy7EErdB

edit: It does show up as a quick fix to import it.

VS Code version: Code 1.33.1 (51b0b28, 2019-04-11T08:22:55.268Z)
OS version: Darwin x64 17.7.0

System Info
Item Value
CPUs Intel(R) Core(TM) i5-6267U CPU @ 2.90GHz (4 x 2900)
GPU Status 2d_canvas: enabled
checker_imaging: disabled_off
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: enabled
rasterization: enabled
surface_synchronization: enabled_on
video_decode: enabled
webgl: enabled
webgl2: enabled
Load (avg) 6, 4, 4
Memory (System) 8.00GB (0.03GB free)
Process Argv
Screen Reader no
VM 0%
Extensions (43)
Extension Author (truncated) Version
atlascode atl 1.3.0
vscode-custom-css be5 3.0.4
vscode-intelephense-client bme 1.0.14
scratchpads bue 0.0.5
better-phpunit cal 1.4.0
path-intellisense chr 1.4.2
laravel-goto-view cod 1.2.5
bracket-pair-colorizer-2 Coe 0.0.28
vscode-markdownlint Dav 0.26.0
vscode-eslint dba 1.8.2
gitlens eam 9.6.3
vsc-material-theme Equ 2.8.2
carbon-now-sh eri 1.2.0
prettier-vscode esb 1.9.0
git-project-manager fel 1.7.1
php-debug fel 1.13.0
code-runner for 0.9.9
macros ged 1.2.1
rest-client hum 0.21.2
vscode-styled-components jpo 0.0.26
php-namespace-resolver Meh 1.1.7
vscode-language-babel mgm 0.0.22
dotenv mik 1.0.1
vscode-typescript-tslint-plugin ms- 1.0.0
debugger-for-chrome msj 4.11.3
php-docblocker nei 1.8.0
vscode-extension-auto-import Nuc 1.4.3
laravel-blade one 1.20.0
laravel-extension-pack one 0.5.0
laravel5-snippets one 1.5.0
vscode-jest Ort 2.9.2
material-icon-theme PKi 3.7.0
text-power-tools qcz 1.11.1
markdown-preview-theme-one-dark rae 0.0.1
vscode-yaml red 0.4.0
code-settings-sync Sha 3.2.9
mdx sil 0.1.0
laravel-goto-controller ste 0.0.7
vscode-laravel-assist tia 0.1.6
vscode-markdown-link-suggestions Tom 13.0.0
vscode-mdx-preview xyc 0.1.5
vscode-surround yat 1.0.2
markdown-all-in-one yzh 2.3.1

(3 theme extensions excluded)

@RMacfarlane RMacfarlane added the javascript JavaScript support issues label May 6, 2019
@mjbvz
Copy link
Collaborator

mjbvz commented May 6, 2019

If you trigger suggestions and then wait until the details for that suggestion show up, I suspect it the import should be added as expected. @jordyvandomselaar Can you please test this

@jrieken I debugged this issue and believe the root cause is that we are accepting a completion item that is not resolved. This was previously detailed in #63013 but we had to revert that fix as it caused other regressions.

@mjbvz mjbvz added bug Issue identified by VS Code Team member as probable bug suggest IntelliSense, Auto Complete labels May 6, 2019
@jordyvandomselaar
Copy link
Author

Thanks for the reply @mjbvz !

I've tested your suggestion and you're correct, after waiting it does import it.

It does take quite long to load the details, is that something related?

@mjbvz
Copy link
Collaborator

mjbvz commented May 7, 2019

Thank you for checking. The details should never take this long to load. I've opened microsoft/TypeScript#31302 to look into why this is happening

@jordyvandomselaar
Copy link
Author

@mjbvz I see the issue you've opened has been resolved. Could you tell me when it'll hit the insiders edition version? That way I can check if the merged PR fixes this issue =)

@jrieken
Copy link
Member

jrieken commented May 21, 2019

Maybe May, the problem is that commit/accept characters run strictly synchronous and that resolving details is async. Either we make TypeScript/JavaScript compute the extra edits eager or we find a better way of implementing commit characters...

@jrieken
Copy link
Member

jrieken commented May 21, 2019

I have pushed a change that ensure completion items are resolve before inserting them, except for inserting/accepting via commit characters (due to its inherent sync nature). That should fix most issues tho

@mjbvz Not sure what to do about the commit character case. Ideally, completion are "done" before resolve is called, e.g. resolve should only load detail/doc but nothing vital for insertion - the doc kinda suggests that. Not sure how feasible that is for TypeScript?

@mjbvz
Copy link
Collaborator

mjbvz commented May 21, 2019

Thanks. Unfortunately we cannot eagerly resolve TS completion items due to auto imports. It would be costly and very verbose for TS to generate the auto import information before the completion is resolved

jrieken added a commit that referenced this issue May 22, 2019
jrieken added a commit that referenced this issue May 22, 2019
@jrieken
Copy link
Member

jrieken commented May 22, 2019

Turns out the changes aren't good... Resolving completion items while inserting them opens the door for out-of-order typing and worst. E.g. select Foo-suggestion, wait for resolving, user types, resolve stops, VS Code insert Foo at the wrong place...

I have reverted my changes and I have also removed the async-call when clicking an item - accepting a suggestion should be immediate, not delayed by anything.

I will push a change so that the UI reflects if an item is still being resolved or not. I am also considering to call resolve more aggressively, e.g on render and not just on focus.

This is an issue with TypeScript and everything needed to make the edits should come from the provide-call, not via resolve. The only alternative is that we call resolve on all items - which I think is a too big hammer.

@jrieken
Copy link
Member

jrieken commented May 22, 2019

Potential UX for showing that an item is still being resolved. With that a user can know if accepting completions does what it want... Not yet entirely sold on the idea tho (changes are in the joh/suggest-busy branch)

May-22-2019 17-16-25

@mjbvz
Copy link
Collaborator

mjbvz commented May 22, 2019

Hopefully with TS 3.5, users should rarely hit this issue.

If we do want to show progress, a spinner would be more clear in my opinion.

@jrieken
Copy link
Member

jrieken commented May 23, 2019

Using some kind of spinner

May-23-2019 11-02-59

@jrieken
Copy link
Member

jrieken commented May 23, 2019

Hopefully with TS 3.5, users should rarely hit this issue.

That's because they it isn't slow anymore, right? It's not that code actions become part of the suggestion without resolve?

@mjbvz
Copy link
Collaborator

mjbvz commented May 23, 2019

Yes, suggestions should be much faster for projects that use styled-components (or other complex type libraries). We have not changed how suggestions are resolved

@jrieken
Copy link
Member

jrieken commented May 24, 2019

Adding @misolori for some UX inspiration - it's tricky because extensions shouldn't do this in the first place and showing progress for something that isn't vital is hard...

@jrieken jrieken removed this from the May 2019 milestone May 24, 2019
@jrieken jrieken removed their assignment Jun 3, 2019
@jrieken
Copy link
Member

jrieken commented Jun 3, 2019

No action planned from the suggest model side. I have added a doc comment to the resolve-function and ultimately extensions shouldn't do this but have completion items ready for insertion when providing them

@lukewlms
Copy link

lukewlms commented Oct 14, 2020

This comes up constantly in my experience and I hope a fix can be implemented! Here's the detail I filed at: #106361

VSCode Version: 1.49.0-insider
OS Version: MacOS Catalina 10.15.6

Steps to Reproduce:

  1. Start typing an import name that has not yet been imported, to bring up the Autocomplete menu
  2. Quickly hit Tab to autocomplete

Result: Autocomplete happens but variable is not found because auto-import failed

(Auto-import only succeeds if I wait an extra 0.5+ seconds until the Auto-Import suggestion loads.)

Expected: If Auto-Import is turned on then Autocomplete of a variable not in the current file already should always auto-import; probably a Tab press should just bring up the Autocomplete suggestions or should not process until after that, because an autocompleted but not imported item that's going to be invalid seems likely to almost never be what the user wants.

Notes: This took me a long time to diagnose because Auto-import usually just failed and I couldn't figure out why. It seems to me an unexpected and erroneous state if VS Code believes it can find an import, then autocompletes it, then afterward it's not found and invalid.

This also slows down the user every time trying to Autocomplete because it will often be unknown if the import has been imported yet or not; if so then fast autocomplete is possible, if not then the user must wait for the Auto-Import box to show up or if autocomplete Tab press was submitted quickly, go back and autocomplete again, this time waiting for the auto-import box.

Fixing this would probably save millions of times 1 second as autocompletes unexpectedly don't auto-import. Just finally understanding this issue is saving me a lot of frustration.

Thanks for taking a look and for the great app.

Does this issue occur when all extensions are disabled?: Yes

@mjbvz
Copy link
Collaborator

mjbvz commented Feb 2, 2021

Duplicate of #109439. Please give the 1.54 insiders builds a try when they come out later this week and let me know if you still see this issue

@mjbvz mjbvz closed this as completed Feb 2, 2021
@mjbvz mjbvz added the *duplicate Issue identified as a duplicate of another issue(s) label Feb 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug *duplicate Issue identified as a duplicate of another issue(s) javascript JavaScript support issues suggest IntelliSense, Auto Complete
Projects
None yet
Development

No branches or pull requests

5 participants