Skip to content

Commit f28e72d

Browse files
authored
[LiquidDoc] Prevent rendering of hover tooltip for snippets with no renderable content (#729)
## What are you adding in this PR? `{% render 'snippet' %}` Prevents rendering hover tooltip for snippets that: - do not exist - do not have renderable content. **Renderable content**: Currently, this means that it requires a `LiquidDoc` definition with `@param` annotations, though this will soon expand to the description and `@example` as well ## Before you deploy <!-- Delete the checklists you don't need --> <!-- Check changes --> - [ ] This PR includes a new checks or changes the configuration of a check - [ ] I included a minor bump `changeset` - [ ] It's in the `allChecks` array in `src/checks/index.ts` - [ ] I ran `yarn build` and committed the updated configuration files <!-- It might be that a check doesn't make sense in a theme-app-extension context --> <!-- When that happens, the check's config should be updated/overridden in the theme-app-extension config --> <!-- see packages/node/configs/theme-app-extension.yml --> - [ ] If applicable, I've updated the `theme-app-extension.yml` config <!-- Public API changes, new features --> - [ ] I included a minor bump `changeset` - [x] My feature is backward compatible <!-- Bug fixes --> - [x] I included a patch bump `changeset`
2 parents a42459e + 6f1ddbb commit f28e72d

File tree

8 files changed

+44
-30
lines changed

8 files changed

+44
-30
lines changed

.changeset/mighty-flies-join.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme-language-server-common': patch
3+
---
4+
5+
Prevent rendering hover tooltip for snippets that do not renderable content

packages/theme-language-server-common/src/documents/DocumentManager.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ export class DocumentManager {
175175
}),
176176
/** Lazy and only computed once per file version */
177177
getLiquidDoc: memo(async (snippetName: string) => {
178-
if (isError(sourceCode.ast)) return { name: snippetName };
178+
if (isError(sourceCode.ast)) return undefined;
179179

180180
return getSnippetDefinition(sourceCode.ast, snippetName);
181181
}),

packages/theme-language-server-common/src/documents/types.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export type AugmentedJsonSourceCode = _AugmentedSourceCode<SourceCodeType.JSON>;
2424
*/
2525
export type AugmentedLiquidSourceCode = _AugmentedSourceCode<SourceCodeType.LiquidHtml> & {
2626
getSchema: () => Promise<SectionSchema | ThemeBlockSchema | AppBlockSchema | undefined>;
27-
getLiquidDoc: (snippetName: string) => Promise<SnippetDefinition>;
27+
getLiquidDoc: (snippetName: string) => Promise<SnippetDefinition | undefined>;
2828
};
2929

3030
/**

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.spec.ts

+9-3
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,16 @@ describe('Module: RenderSnippetHoverProvider', async () => {
5454
);
5555
});
5656

57-
it('should return an H3 with snippet name if no LiquidDocDefinition found', async () => {
58-
getSnippetDefinition = async () => ({ name: 'unknown-snippet' });
57+
it('should return null if no LiquidDocDefinition found', async () => {
58+
getSnippetDefinition = async () => ({ name: 'unknown-snippet', liquidDoc: undefined });
5959
provider = createProvider(getSnippetDefinition);
60-
await expect(provider).to.hover(`{% render 'unknown-sni█ppet' %}`, '### unknown-snippet');
60+
await expect(provider).to.hover(`{% render 'unknown-sni█ppet' %}`, `### unknown-snippet`);
61+
});
62+
63+
it('should return null if snippet is null', async () => {
64+
getSnippetDefinition = async () => undefined;
65+
provider = createProvider(getSnippetDefinition);
66+
await expect(provider).to.hover(`{% render 'unknown-sni█ppet' %}`, null);
6167
});
6268

6369
it('should return nothing if not in render tag', async () => {

packages/theme-language-server-common/src/hover/providers/RenderSnippetHoverProvider.ts

+5-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
99
private getSnippetDefinitionForURI: (
1010
uri: string,
1111
snippetName: string,
12-
) => Promise<SnippetDefinition>,
12+
) => Promise<SnippetDefinition | undefined>,
1313
) {}
1414

1515
async hover(
@@ -32,6 +32,10 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
3232
snippetName,
3333
);
3434

35+
if (!snippetDefinition) {
36+
return null;
37+
}
38+
3539
const liquidDoc = snippetDefinition.liquidDoc;
3640

3741
if (!liquidDoc) {

packages/theme-language-server-common/src/liquidDoc.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
99
return toSourceCode('/tmp/foo.liquid', code).ast as LiquidHtmlNode;
1010
}
1111

12-
it('should return name if no valid annotations are present in definition', async () => {
12+
it('should return default snippet definition if no renderable content is present', async () => {
1313
const ast = toAST(`
1414
{% doc %}
1515
just a description

packages/theme-language-server-common/src/liquidDoc.ts

+19-19
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,7 @@ import { LiquidDocParamNode } from '@shopify/liquid-html-parser';
77
export type GetSnippetDefinitionForURI = (
88
uri: string,
99
snippetName: string,
10-
) => Promise<SnippetDefinition>;
11-
12-
export type LiquidDocParameter = {
13-
name: string;
14-
description: string | null;
15-
type: string | null;
16-
};
10+
) => Promise<SnippetDefinition | undefined>;
1711

1812
export type SnippetDefinition = {
1913
name: string;
@@ -24,27 +18,33 @@ type LiquidDocDefinition = {
2418
parameters?: LiquidDocParameter[];
2519
};
2620

21+
export type LiquidDocParameter = {
22+
name: string;
23+
description: string | null;
24+
type: string | null;
25+
};
26+
2727
export function getSnippetDefinition(
2828
snippet: LiquidHtmlNode,
2929
snippetName: string,
3030
): SnippetDefinition {
31-
const liquidDocParameters: LiquidDocParameter[] = visit<
32-
SourceCodeType.LiquidHtml,
33-
LiquidDocParameter
34-
>(snippet, {
35-
LiquidDocParamNode(node: LiquidDocParamNode) {
36-
return {
37-
name: node.paramName.value,
38-
description: node.paramDescription?.value ?? null,
39-
type: node.paramType?.value ?? null,
40-
};
31+
const parameters: LiquidDocParameter[] = visit<SourceCodeType.LiquidHtml, LiquidDocParameter>(
32+
snippet,
33+
{
34+
LiquidDocParamNode(node: LiquidDocParamNode) {
35+
return {
36+
name: node.paramName.value,
37+
description: node.paramDescription?.value ?? null,
38+
type: node.paramType?.value ?? null,
39+
};
40+
},
4141
},
42-
});
42+
);
4343

4444
return {
4545
name: snippetName,
4646
liquidDoc: {
47-
parameters: liquidDocParameters,
47+
parameters,
4848
},
4949
};
5050
}

packages/theme-language-server-common/src/server/startServer.ts

+3-4
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ import { snippetName } from '../utils/uri';
4343
import { VERSION } from '../version';
4444
import { CachedFileSystem } from './CachedFileSystem';
4545
import { Configuration } from './Configuration';
46-
import { LiquidHtmlNode } from '@shopify/liquid-html-parser';
47-
import { getSnippetDefinition, SnippetDefinition } from '../liquidDoc';
46+
import { SnippetDefinition } from '../liquidDoc';
4847

4948
const defaultLogger = () => {};
5049

@@ -176,13 +175,13 @@ export function startServer(
176175
const getSnippetDefinitionForURI = async (
177176
uri: string,
178177
snippetName: string,
179-
): Promise<SnippetDefinition> => {
178+
): Promise<SnippetDefinition | undefined> => {
180179
const rootUri = await findThemeRootURI(uri);
181180
const snippetURI = path.join(rootUri, 'snippets', `${snippetName}.liquid`);
182181
const snippet = documentManager.get(snippetURI);
183182

184183
if (!snippet || snippet.type !== SourceCodeType.LiquidHtml || isError(snippet.ast)) {
185-
return { name: snippetName };
184+
return undefined;
186185
}
187186

188187
return snippet.getLiquidDoc(snippetName);

0 commit comments

Comments
 (0)