Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/flat-geckos-breathe.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme-language-server-common': minor
---

Added completion support for inline snippets in the `render` tag
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,23 @@ describe('Module: RenderSnippetCompletionProvider', async () => {

it('should complete snippets correctly', async () => {
await expect(provider).to.complete('{% render "', ['product-card', 'image']);
await expect(provider).to.complete('{% render "product', [
// VSCode handles filtering
});

it('should complete inline snippets', async () => {
const template = `{% snippet my-inline-snippet %}
{% echo 'hello' %}
{% endsnippet %}

{% render m█ %}`;

await expect(provider).to.complete(template, ['my-inline-snippet']);
// VSCode handles filtering
await expect(provider).to.complete(template, [
expect.objectContaining({
documentation: {
kind: 'markdown',
value: 'snippets/product-card.liquid',
value: `Inline snippet "my-inline-snippet"`,
},
}),
]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { NodeTypes } from '@shopify/liquid-html-parser';
import { LiquidHtmlNode, LiquidTag, NodeTypes } from '@shopify/liquid-html-parser';
import { CompletionItem, CompletionItemKind } from 'vscode-languageserver';
import { LiquidCompletionParams } from '../params';
import { Provider } from './common';
import { SourceCodeType, visit, isError } from '@shopify/theme-check-common';
import { findInlineSnippetAncestor } from '@shopify/theme-check-common/dist/checks/utils';

export type GetSnippetNamesForURI = (uri: string) => Promise<string[]>;

Expand All @@ -14,21 +16,13 @@ export class RenderSnippetCompletionProvider implements Provider {
const { node, ancestors } = params.completionContext;
const parentNode = ancestors.at(-1);

if (
!node ||
!parentNode ||
node.type !== NodeTypes.String ||
parentNode.type !== NodeTypes.RenderMarkup
) {
if (!node || !parentNode || parentNode.type !== NodeTypes.RenderMarkup) {
return [];
}

const options = await this.getSnippetNamesForURI(params.textDocument.uri);
const partial = node.value;

return options
.filter((option) => option.startsWith(partial))
.map(
if (node.type === NodeTypes.String) {
const fileSnippets = await this.getSnippetNamesForURI(params.textDocument.uri);
const fileCompletionItems = fileSnippets.map(
(option: string): CompletionItem => ({
label: option,
kind: CompletionItemKind.Snippet,
Expand All @@ -38,5 +32,42 @@ export class RenderSnippetCompletionProvider implements Provider {
},
}),
);
return fileCompletionItems;
} else if (node.type === NodeTypes.VariableLookup) {
const containingSnippet = findInlineSnippetAncestor(ancestors);
const containingSnippetName =
containingSnippet && typeof containingSnippet.markup !== 'string'
? containingSnippet.markup.name
: null;

const fullAst = params.document.ast;
const allInlineSnippets = isError(fullAst) ? [] : getInlineSnippetsNames(fullAst);

const inlineSnippets = allInlineSnippets.filter((name) => name !== containingSnippetName);

const inlineCompletionItems = inlineSnippets.map(
(option: string): CompletionItem => ({
label: option,
kind: CompletionItemKind.Snippet,
documentation: {
kind: 'markdown',
value: `Inline snippet "${option}"`,
},
}),
);
return inlineCompletionItems;
} else {
return [];
}
}
}

function getInlineSnippetsNames(ast: LiquidHtmlNode): string[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Another scope/nesting question: do we want to limit the options to "current level" snippets? For example, should the below only offer header_inner?

example Image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooh yeah you're right! Thanks for catching that 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I'm wondering if we should include it in other snippets? ie:

image.png

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah this is tricky, hi-snip could be passed in via render to hi-snip-2, but should tooling expect that? Maybe we could just limit completion to prevent a snippet being offered inside itself and leave the rest as is?

@aswamy do you think that would make sense?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could just limit completion to prevent a snippet being offered inside itself and leave the rest as is?

I think you're right. For simplicity, we should only offer code completion to things that are explicitly defined within the snippet.

This includes ALL variables. The reason is that we aren't sure how the snippet is rendered, which can get very confusing.

return visit<SourceCodeType.LiquidHtml, string>(ast, {
LiquidTag(node: LiquidTag) {
if (node.name === 'snippet' && typeof node.markup !== 'string' && node.markup.name) {
return node.markup.name;
}
},
});
}