fix: Prevent excessive autocompletion in template JS files #490
Conversation
We shouldn't be checking for curly braces yet in JS files, so this fix ignores them entirely until we explictly want to check for something. Updated tests to explictly test JS files as well. vscode #3902
| @@ -141,7 +141,7 @@ export default class Server { | |||
| if (await this.context.isLWCTemplate(doc)) { | |||
| this.auraDataProvider.activated = false; // provide completions for lwc components in an Aura template | |||
| this.lwcDataProvider.activated = true; | |||
| if (params.context?.triggerCharacter === '{') { | |||
| if (this.shouldProvideBindingsInHTML(params)) { | |||
There was a problem hiding this comment.
Open to better naming here and for the shouldCompleteJavascript below, but I definitely didn't think the previous statement was clear on why we were matching on specific characters.
| }, | ||
| context: { | ||
| triggerCharacter: '{', | ||
| triggerKind: 2, |
There was a problem hiding this comment.
total nit: do we have an enum for triggerKind?
There was a problem hiding this comment.
So this comes from the vscode-language-server-protocol, where it looks like CompletionTriggerKind is declared as a type. Not sure if we can/want to import that here.
There was a problem hiding this comment.
ah gotcha..yeah looks like we can just import and use that type.
There was a problem hiding this comment.
Cool beans, just added!
| isIncomplete: false, | ||
| items: customTags, | ||
| }; | ||
| } |
There was a problem hiding this comment.
so previously we always returned here, but we only return when it passes shouldCompleteJavascript. Should we move the inner if into the above condition, or should we be returning with no results when no matching tag was found?
There was a problem hiding this comment.
Ah, I see what you mean. I think an early empty return like what we have for the last else would be a good change here.
There was a problem hiding this comment.
things that make you go hummmmmmmmm. How does undefined qualify as a CompletionList?
weird.
I did a little digging and think it's b/c CompletionList is a namespace and an interface (maybe). sometimes I wonder if the teams at microsoft talk to each other :). eh. def better thanks for the update.
What does this PR do?
What issues does this PR fix or reference?
VS Code #3902. @W-10832653@