Skip to content

Commit ea9f245

Browse files
karreirokarreiro
committed
Reintroduce support for context parameters in the content_for tag
1 parent d9b5815 commit ea9f245

File tree

6 files changed

+36
-60
lines changed

6 files changed

+36
-60
lines changed

‎.changeset/old-cars-sort.md

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@shopify/theme-language-server-common': minor
3+
'@shopify/theme-check-common': minor
4+
---
5+
6+
Reintroduce support for `context` parameters in the `content_for` tag

‎packages/theme-check-common/src/checks/valid-content-for-arguments/index.spec.ts

+8-30
Original file line numberDiff line numberDiff line change
@@ -22,29 +22,22 @@ describe('Module: ValidContentForArguments', () => {
2222
expect(offenses).toHaveLength(0);
2323
});
2424

25-
it('should report offenses for non-`closest.*` kwargs', async () => {
25+
it('should accept `context.*` kwargs', async () => {
2626
const offenses = await runLiquidCheck(
2727
ValidContentForArguments,
28-
'{% content_for "blocks", product: product %}',
29-
);
30-
expect(offenses).toHaveLength(1);
31-
expect(offenses[0]!.message).to.equal(
32-
`{% content_for "blocks" %} only accepts 'closest.*' arguments`,
28+
'{% content_for "blocks", context.product: product %}',
3329
);
30+
expect(offenses).toHaveLength(0);
3431
});
3532

36-
it('should report offenses for deprecated `context.*` kwargs', async () => {
33+
it('should report offenses for non-`closest.*` kwargs', async () => {
3734
const offenses = await runLiquidCheck(
3835
ValidContentForArguments,
39-
'{% content_for "blocks", context.product: product %}',
36+
'{% content_for "blocks", product: product %}',
4037
);
41-
expect(offenses).toHaveLength(2);
38+
expect(offenses).toHaveLength(1);
4239
expect(offenses[0]!.message).to.equal(
43-
`{% content_for "blocks" %} only accepts 'closest.*' arguments`,
44-
);
45-
46-
expect(offenses[1]!.message).to.equal(
47-
`{% content_for "blocks" %} only accepts 'closest.*' arguments. The 'context.*' arguments usage has been deprecated.`,
40+
`{% content_for "blocks" %} only accepts 'closest.*' and 'context.*' arguments`,
4841
);
4942
});
5043
});
@@ -83,22 +76,7 @@ describe('Module: ValidContentForArguments', () => {
8376
);
8477
expect(offenses).toHaveLength(1);
8578
expect(offenses[0].message).to.equal(
86-
`{% content_for "block" %} only accepts 'id', 'type' and 'closest.*' arguments`,
87-
);
88-
});
89-
90-
it('should report offenses for deprecated `context.*` kwargs', async () => {
91-
const offenses = await runLiquidCheck(
92-
ValidContentForArguments,
93-
'{% content_for "block", type: "type", id: "static-block", context.product: product %}',
94-
);
95-
expect(offenses).toHaveLength(2);
96-
expect(offenses[0]!.message).to.equal(
97-
`{% content_for "block" %} only accepts 'id', 'type' and 'closest.*' arguments`,
98-
);
99-
100-
expect(offenses[1]!.message).to.equal(
101-
`{% content_for "block" %} accepts 'closest.*' arguments. The 'context.*' arguments usage has been deprecated.`,
79+
`{% content_for "block" %} only accepts 'id', 'type', 'closest.*', and 'context.*' arguments`,
10280
);
10381
});
10482
});

‎packages/theme-check-common/src/checks/valid-content-for-arguments/index.ts

+7-25
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { ContentForMarkup, NodeTypes } from '@shopify/liquid-html-parser';
22
import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
33

4-
// content_for "block" and content_for "blocks" only allow `closest.*` kwargs.
5-
const isClosestArgument = (argName: string) => argName.startsWith('closest.');
6-
const isContextArgument = (argName: string) => argName.startsWith('context.');
4+
const isValidKeywordArgument = (argName: string) => {
5+
return argName.startsWith('closest.') || argName.startsWith('context.');
6+
};
77

88
export const ValidContentForArguments: LiquidCheckDefinition = {
99
meta: {
@@ -24,19 +24,10 @@ export const ValidContentForArguments: LiquidCheckDefinition = {
2424
create(context) {
2525
const validationStrategies = {
2626
blocks: (node: ContentForMarkup) => {
27-
const problematicArguments = node.args.filter((arg) => !isClosestArgument(arg.name));
27+
const problematicArguments = node.args.filter((arg) => !isValidKeywordArgument(arg.name));
2828
for (const arg of problematicArguments) {
2929
context.report({
30-
message: `{% content_for "blocks" %} only accepts 'closest.*' arguments`,
31-
startIndex: arg.position.start,
32-
endIndex: arg.position.end,
33-
});
34-
}
35-
36-
const deprecatedArguments = node.args.filter((arg) => isContextArgument(arg.name));
37-
for (const arg of deprecatedArguments) {
38-
context.report({
39-
message: `{% content_for "blocks" %} only accepts 'closest.*' arguments. The 'context.*' arguments usage has been deprecated.`,
30+
message: `{% content_for "blocks" %} only accepts 'closest.*' and 'context.*' arguments`,
4031
startIndex: arg.position.start,
4132
endIndex: arg.position.end,
4233
});
@@ -72,21 +63,12 @@ export const ValidContentForArguments: LiquidCheckDefinition = {
7263
}
7364

7465
const problematicArguments = node.args.filter(
75-
(arg) => !(requiredArguments.includes(arg.name) || isClosestArgument(arg.name)),
66+
(arg) => !(requiredArguments.includes(arg.name) || isValidKeywordArgument(arg.name)),
7667
);
7768

7869
for (const arg of problematicArguments) {
7970
context.report({
80-
message: `{% content_for "block" %} only accepts 'id', 'type' and 'closest.*' arguments`,
81-
startIndex: arg.position.start,
82-
endIndex: arg.position.end,
83-
});
84-
}
85-
86-
const deprecatedArguments = node.args.filter((arg) => isContextArgument(arg.name));
87-
for (const arg of deprecatedArguments) {
88-
context.report({
89-
message: `{% content_for "block" %} accepts 'closest.*' arguments. The 'context.*' arguments usage has been deprecated.`,
71+
message: `{% content_for "block" %} only accepts 'id', 'type', 'closest.*', and 'context.*' arguments`,
9072
startIndex: arg.position.start,
9173
endIndex: arg.position.end,
9274
});

‎packages/theme-language-server-common/src/completions/providers/ContentForParameterCompletionProvider.spec.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ describe('Module: ContentForBlockParameterCompletionProvider', async () => {
4545
'type',
4646
'id',
4747
'closest',
48+
'context',
4849
'testOption',
4950
]);
5051
});
@@ -109,9 +110,9 @@ describe('Module: ContentForBlockParameterCompletionProvider', async () => {
109110
);
110111
});
111112

112-
describe("when we're completing for blocks we only allow `closest`", () => {
113+
describe("when we're completing for blocks we allow `closest` and `context`", () => {
113114
it('does something', async () => {
114-
await expect(provider).to.complete('{% content_for "blocks", █ %}', ['closest']);
115+
await expect(provider).to.complete('{% content_for "blocks", █ %}', ['closest', 'context']);
115116
});
116117
});
117118

@@ -173,7 +174,13 @@ describe('Module: ContentForBlockParameterCompletionProvider', async () => {
173174
it('offers a full list of completion items', async () => {
174175
const context = `{% content_for "block", █type: "button" %}`;
175176

176-
await expect(provider).to.complete(context, ['type', 'id', 'closest', 'testOption']);
177+
await expect(provider).to.complete(context, [
178+
'type',
179+
'id',
180+
'closest',
181+
'context',
182+
'testOption',
183+
]);
177184
});
178185

179186
it('does not replace the existing text', async () => {

‎packages/theme-language-server-common/src/completions/providers/ContentForParameterCompletionProvider.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ export class ContentForParameterCompletionProvider implements Provider {
4444
if (parentNode.contentForType.value == 'blocks') {
4545
options = {
4646
closest: DEFAULT_COMPLETION_OPTIONS.closest,
47+
context: DEFAULT_COMPLETION_OPTIONS.context,
4748
};
4849
}
4950

@@ -86,8 +87,9 @@ export class ContentForParameterCompletionProvider implements Provider {
8687

8788
let start = document.textDocument.positionAt(node.position.start);
8889
let end = document.textDocument.positionAt(node.position.end + offset);
89-
let newText = name === 'closest' ? `${name}.` : `${name}: '$1'`;
90-
let format = name === 'closest' ? InsertTextFormat.PlainText : InsertTextFormat.Snippet;
90+
let isValidKeywordArg = name === 'closest' || name === 'context';
91+
let newText = isValidKeywordArg ? `${name}.` : `${name}: '$1'`;
92+
let format = isValidKeywordArg ? InsertTextFormat.PlainText : InsertTextFormat.Snippet;
9193

9294
// If the cursor is inside the parameter or at the end and it's the same
9395
// value as the one we're offering a completion for then we want to restrict

‎packages/theme-language-server-common/src/completions/providers/data/contentForParameterCompletionOptions.ts

+1
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ export const DEFAULT_COMPLETION_OPTIONS: { [key: string]: string } = {
66
type: "The type (name) of an existing theme block in your theme’s /blocks folder. Only applicable when `content_type` is 'block'.",
77
id: "A unique identifier and literal string within the section or block that contains the static blocks. Only applicable when `content_type` is 'block'.",
88
closest: 'A path that provides a way to access the closest resource of a given type.',
9+
context: 'A path that provides a way to pass context properties to blocks.',
910
};

0 commit comments

Comments
 (0)