Skip to content

Commit a7e2320

Browse files
committed
Wrap optional parameters name in [] in hover tooltip
----- Behaviour diverges slightly from JSDoc here. JSDoc does not indicate that it's optional, rather it implicitly adds `| undefined` to the types
1 parent 3c791ea commit a7e2320

File tree

4 files changed

+40
-9
lines changed

4 files changed

+40
-9
lines changed

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -16,26 +16,31 @@ describe('Module: RenderSnippetHoverProvider', async () => {
1616
name: 'title',
1717
description: 'The title of the product',
1818
type: 'string',
19+
required: true,
1920
},
2021
{
2122
name: 'border-radius',
2223
description: 'The border radius in px',
2324
type: 'number',
25+
required: false,
2426
},
2527
{
2628
name: 'no-type',
2729
description: 'This parameter has no type',
2830
type: null,
31+
required: true,
2932
},
3033
{
3134
name: 'no-description',
3235
description: null,
3336
type: 'string',
37+
required: true,
3438
},
3539
{
3640
name: 'no-type-or-description',
3741
description: null,
3842
type: null,
43+
required: true,
3944
},
4045
],
4146
},
@@ -66,7 +71,7 @@ describe('Module: RenderSnippetHoverProvider', async () => {
6671
it('should return snippet definition with all parameters', async () => {
6772
await expect(provider).to.hover(
6873
`{% render 'product-car█d' %}`,
69-
'### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `border-radius`: number - The border radius in px\n- `no-type` - This parameter has no type\n- `no-description`: string\n- `no-type-or-description`',
74+
'### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `[border-radius]`: number - The border radius in px\n- `no-type` - This parameter has no type\n- `no-description`: string\n- `no-type-or-description`',
7075
);
7176
});
7277

@@ -86,5 +91,12 @@ describe('Module: RenderSnippetHoverProvider', async () => {
8691
await expect(provider).to.hover(`{% assign asdf = 'snip█pet' %}`, null);
8792
await expect(provider).to.hover(`{{ 'snip█pet' }}`, null);
8893
});
94+
95+
it('should return snippet definition with all parameters', async () => {
96+
await expect(provider).to.hover(
97+
`{% render 'product-car█d' %}`,
98+
'### product-card\n\n**Parameters:**\n- `title`: string - The title of the product\n- `[border-radius]`: number - The border radius in px\n- `no-type` - This parameter has no type\n- `no-description`: string\n- `no-type-or-description`',
99+
);
100+
});
89101
});
90102
});

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

+12-7
Original file line numberDiff line numberDiff line change
@@ -50,13 +50,7 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
5050
const parts = [`### ${snippetDefinition.name}`];
5151

5252
if (liquidDoc.parameters?.length) {
53-
const parameters = liquidDoc.parameters
54-
?.map(
55-
({ name, type, description }: LiquidDocParameter) =>
56-
`- \`${name}\`${type ? `: ${type}` : ''}${description ? ` - ${description}` : ''}`,
57-
)
58-
.join('\n');
59-
53+
const parameters = this.buildParameters(liquidDoc.parameters);
6054
parts.push('', '**Parameters:**', parameters);
6155
}
6256

@@ -67,4 +61,15 @@ export class RenderSnippetHoverProvider implements BaseHoverProvider {
6761
},
6862
};
6963
}
64+
65+
private buildParameters(parameters: LiquidDocParameter[]) {
66+
return parameters
67+
.map(({ name, type, description, required }: LiquidDocParameter) => {
68+
const nameStr = required ? `\`${name}\`` : `\`[${name}]\``;
69+
const typeStr = type ? `: ${type}` : '';
70+
const descStr = description ? ` - ${description}` : '';
71+
return `- ${nameStr}${typeStr}${descStr}`;
72+
})
73+
.join('\n');
74+
}
7075
}

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

+13-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { toSourceCode } from '@shopify/theme-check-common';
44
import { describe } from 'vitest';
55
import { getSnippetDefinition } from './liquidDoc';
66

7-
describe('Unit: makeGetLiquidDocDefinitions', () => {
7+
describe('Unit: getSnippetDefinition', () => {
88
function toAST(code: string) {
99
return toSourceCode('/tmp/foo.liquid', code).ast as LiquidHtmlNode;
1010
}
@@ -31,6 +31,7 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
3131
{% doc %}
3232
@param {String} firstParam - The first param
3333
@param {Number} secondParam - The second param
34+
@param {String} [optionalParam] - The optional param
3435
@param paramWithNoType - param with no type
3536
@param paramWithOnlyName
3637
@param {Number} paramWithNoDescription
@@ -46,26 +47,37 @@ describe('Unit: makeGetLiquidDocDefinitions', () => {
4647
name: 'firstParam',
4748
description: 'The first param',
4849
type: 'String',
50+
required: true,
4951
},
5052
{
5153
name: 'secondParam',
5254
description: 'The second param',
5355
type: 'Number',
56+
required: true,
57+
},
58+
{
59+
name: 'optionalParam',
60+
description: 'The optional param',
61+
type: 'String',
62+
required: false,
5463
},
5564
{
5665
name: 'paramWithNoType',
5766
description: 'param with no type',
5867
type: null,
68+
required: true,
5969
},
6070
{
6171
name: 'paramWithOnlyName',
6272
description: null,
6373
type: null,
74+
required: true,
6475
},
6576
{
6677
name: 'paramWithNoDescription',
6778
description: null,
6879
type: 'Number',
80+
required: true,
6981
},
7082
],
7183
},

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

+2
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export type LiquidDocParameter = {
2222
name: string;
2323
description: string | null;
2424
type: string | null;
25+
required: boolean;
2526
};
2627

2728
export function getSnippetDefinition(
@@ -36,6 +37,7 @@ export function getSnippetDefinition(
3637
name: node.paramName.value,
3738
description: node.paramDescription?.value ?? null,
3839
type: node.paramType?.value ?? null,
40+
required: node.required,
3941
};
4042
},
4143
},

0 commit comments

Comments
 (0)