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

TypeScript import from suggestions no longer works after enabling lit-plugin #200

Closed
Artur- opened this issue Sep 16, 2021 · 21 comments · Fixed by #206
Closed

TypeScript import from suggestions no longer works after enabling lit-plugin #200

Artur- opened this issue Sep 16, 2021 · 21 comments · Fixed by #206

Comments

@Artur-
Copy link

Artur- commented Sep 16, 2021

lit-plugin-enabled.mov
lit-plugin-disabled.mov
@Artur- Artur- changed the title TypeScript import auto completion no longer works after enabling this extension TypeScript import auto completion no longer works after enabling lit-plugin Sep 16, 2021
@Artur- Artur- changed the title TypeScript import auto completion no longer works after enabling lit-plugin TypeScript import from suggestions no longer works after enabling lit-plugin Sep 16, 2021
@jjspace
Copy link

jjspace commented Sep 16, 2021

Just commenting to make sure this is linked back to the original issue opened in the VSCode repo which may have more information. microsoft/vscode#132299

Also this is specifically only happening on version 1.60+ of VSCode, everything was working fine in version 1.59 so something changed with that release.

I can also confirm that this happens for normal .js files as well, it's not limited to typescript

@rick-yo
Copy link

rick-yo commented Sep 18, 2021

Same problem happens on my vscode. version 1.60.1

@Kokujou
Copy link

Kokujou commented Sep 21, 2021

same problem for me. just wanted to add: there is no specific extension configuration, all other extensions have been uninstalled to verify that it is really this extensions. further it is not about typescript alone.
the project i'm working with is javascript based and is only using relative imports (no node_modules or NPM imports)

@Artur-
Copy link
Author

Artur- commented Sep 21, 2021

This is from the (closed) VS Code issue about how it can be reproduced: microsoft/vscode#132299 (comment)

git clone https://github.com/Artur-/vscode-import-problem
cd vscode-import-problem
code .
  1. Open both exporter.js and importer.js
  2. In importer.js auto complete the import for exportedMethod

With lit-plugin installed and enabled: No import is added
With lit-plugin disabled: The import is added as expected

@Artur-
Copy link
Author

Artur- commented Sep 21, 2021

The only thing I found while digging around is that this could possibly related to getCompletionEntryDetails having an additional parameter in TS 4.4 that is not present at least in 4.2

getCompletionEntryDetails(
fileName: string,
position: number,
name: string,
formatOptions: FormatCodeOptions | FormatCodeSettings | undefined,
source: string | undefined,
preferences: UserPreferences | undefined
): CompletionEntryDetails | undefined {
const file = this.program.getSourceFile(fileName)!;
const result = this.litAnalyzer.getCompletionDetailsAtPosition(file, position, name);
return (
(result && translateCompletionDetails(result, this.context)) ||
this.prevLangService.getCompletionEntryDetails(fileName, position, name, formatOptions, source, preferences)
);
}
is what is causing the problem

@43081j
Copy link
Contributor

43081j commented Sep 28, 2021

@justinfagnani i think artur may be right. 4.x of typescript brought in an extra parameter, data, to the getCompletionEntryDetails.

i suppose if we're using the user's local typescript, they could be on 4.x which would lead to the data parameter being lost when it goes through the code here? as we'd pass through to prevLangService without that parameter.

in order to update lit-analyzer to ts 4.x, though, i think we need to update web-component-analyzer first.

this was the commit to typescript:
microsoft/TypeScript@993c503

@Kokujou
Copy link

Kokujou commented Oct 13, 2021

so what do you think, will there be a fix soon? do you need to assign some responsible person for this bug?
i would think the priority is kinda high because it breaks an important functionality.
for me and my team it would gain more and more importance.

@43081j
Copy link
Contributor

43081j commented Oct 13, 2021

web-component-analyzer possibly needs updating to typescript 4.4, thats here.

lit-analyzer then needs updating to pull that in, and itself upgrade typescript, and pass the new parameter through.

i'll have another look in the next couple of days to see if i can get a branch ready of lit-analyzer too, but it may be blocked by the WCA PR.

the most likely person to merge/release this stuff is @justinfagnani

edit: looks like we may also need to upgrade typescript in ts-simple-type 🤦 as that wants to be aligned with the one we install here

edit2: tracking in #206. ill update that PR once WCA and ts-simple-type are updated

@vdegenne
Copy link

vdegenne commented Dec 4, 2021

Any updates? There is a pending pull request.
This issue is driving me nut, I have to import all symbols manually.

@Kokujou
Copy link

Kokujou commented Dec 4, 2021

well you could also disable this extension instead...
or, if you want to be very clever, you can install the extension manually from the branch the PR is on^^

@43081j
Copy link
Contributor

43081j commented Dec 6, 2021

We're waiting on runem/web-component-analyzer#236, justin and peter are pretty busy with lit and i don't have access to merge/publish, so unfortunately i can't do much yet.

if the new WCA version gets published, we can then merge #206 i think

@jjspace
Copy link

jjspace commented Dec 24, 2021

@43081j any way we can move this along? We're now 3 versions behind VSCode (being locked to v1.59 so this works) and I'd really like to update but I use lit-plugin all day every day so I don't feel comfortable going to 1.60+ until this is fixed...

@43081j
Copy link
Contributor

43081j commented Dec 24, 2021

Unfortunately we're still blocked until @justinfagnani can find some time to merge the related PRs and publish a new version (I don't have permission).

I'll see if I can catch up with him and Peter again after Christmas and get this sorted

@jjspace
Copy link

jjspace commented Mar 9, 2022

Bump, another 2 months with little movement... Am I correct that this is just waiting on #206 which is in turn waiting on a deploy of web-components-analyzer? Is that where we're at? everything is in place it just needs to get released/merged?

I'm genuinely confused/interested how other people work with Lit without a tool like this to help them see syntax and have completions. How do the developers of Lit itself even work with it? Or are we all just putting up with the out of date plugin on an old version of VSCode like I am...? Or maybe just putting up with manually importing?

CC @43081j @rictic @justinfagnani

@Kokujou
Copy link

Kokujou commented Mar 9, 2022

honestly if it' sjust waiting for a simple PR to be merged and the code is already there, i wish someone would just clone this repo and make a temporary release until this stuff is properly integrated into the release. but maybe i'm not understanding what exactly is missing.

@blikblum
Copy link

blikblum commented Mar 9, 2022

I'm genuinely confused/interested how other people work with Lit

I removed this plugin. Really miss it. Even with this plugin working, the IDE support to Lit ecosystem is subpar compared to e.g. React or Angular. Its one of the drawbacks of using Lit

@43081j
Copy link
Contributor

43081j commented Mar 10, 2022

im really sorry for the slowness here.

what we are waiting on:

there's nothing difficult left to do as far as im aware. i don't have permissions to merge and publish these things myself, so im waiting on @rictic or @justinfagnani to do it. both are pretty busy people though so progress hasn't been great.

i will try catch up with them again soon if i can get hold of them, and maybe we can get this stuff finally merged and sorted

@jjspace
Copy link

jjspace commented Apr 4, 2022

Thanks so much for your work on this @rictic and @43081j! 🙏 Looking forward to a release of the plugin to roll out so I can finally upgrade my vscode 🎉

@rictic
Copy link
Collaborator

rictic commented Apr 26, 2022

Forgot to update here last week but the updated version is now released!

@vdegenne
Copy link

vdegenne commented May 4, 2022

Are auto-closing features not working or are they just not implemented?
I was wishing this update would fix that but it doesn't (even though auto-import works now) or am I just missing something?

By auto-closing Here's an instance of what I mean :
In a string literal if I start to type dis the auto-completion box opens and offer me to select display to go faster.
If I select it, it auto-completes the term but not the punctuations as it does in a dedicated file :

What I get in an element context (after completion) :

static styles = css`
#header {
  display<caret>
}
`

What I get in an html file (after completion) :

<style>
#header {
  display:<caret>;
}
</style>

Wouldn't that be easy to implement? If it's just a matter of forwarding an attribute from the IDE settings to the plugin parser.

@43081j
Copy link
Contributor

43081j commented May 5, 2022

interesting, we don't interfere with that stuff as far as i know so it could be some state we're losing.

ill see if i can reproduce it in a test this weekend

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 a pull request may close this issue.

8 participants