Skip to content

Commit 673eafd

Browse files
lukeh-shopifylukeh-shopify
authored
Merge pull request #1281 from Shopify/lh-standard-event-data-check
Add check for standard_event_data
2 parents e7d1d05 + c141326 commit 673eafd

6 files changed

Lines changed: 315 additions & 0 deletions

File tree

‎.changeset/valid-standard-event-data.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@shopify/theme-check-common': minor
3+
---
4+
5+
Add `ValidStandardEventData` check to error on invalid arguments to the `standard_event_data` filter.
6+
7+
The filter's argument values are currently only validated at render time. This check catches invalid literal values statically: `view` is the only supported event type, and `context:` must be one of `page`, `search`, `collection`, `dialog`, or `recommendation`.
8+
9+
Values that aren't string literals are left alone, since the type of the piped input isn't statically knowable.

‎packages/theme-check-common/src/checks/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import { ValidSchema } from './valid-schema';
6464
import { ValidSchemaName } from './valid-schema-name';
6565
import { ValidSchemaTranslations } from './valid-schema-translations';
6666
import { ValidSettingsKey } from './valid-settings-key';
67+
import { ValidStandardEventData } from './valid-standard-event-data';
6768
import { ValidStaticBlockType } from './valid-static-block-type';
6869
import { ValidVisibleIf, ValidVisibleIfSettingsSchema } from './valid-visible-if';
6970
import { VariableName } from './variable-name';
@@ -154,6 +155,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [
154155
ValidRenderSnippetArgumentTypes,
155156
ValidSchema,
156157
ValidSettingsKey,
158+
ValidStandardEventData,
157159
ValidStaticBlockType,
158160
ValidVisibleIf,
159161
ValidVisibleIfSettingsSchema,

‎packages/theme-check-common/src/checks/valid-standard-event-data/index.spec.ts‎

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { highlightedOffenses, runLiquidCheck } from '../../test';
3+
import { ValidStandardEventData } from './index';
4+
5+
describe('Module: ValidStandardEventData', () => {
6+
it('reports an offense when the context is not a supported value for products', async () => {
7+
const sourceCode = `{{ product | standard_event_data: 'view', context: 'homepage' }}`;
8+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
9+
10+
expect(offenses).toHaveLength(1);
11+
expect(offenses[0].message).toBe(
12+
"Unsupported context 'homepage' for product. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.",
13+
);
14+
15+
const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses);
16+
expect(highlights[0]).to.eql("'homepage'");
17+
});
18+
19+
it('reports an offense when the context is not a supported value for carts', async () => {
20+
const sourceCode = `{{ cart | standard_event_data: 'view', context: 'banner' }}`;
21+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
22+
23+
expect(offenses).toHaveLength(1);
24+
expect(offenses[0].message).toBe(
25+
"Unsupported context 'banner' for cart. Valid values: page, dialog. The 'context' argument can also be omitted.",
26+
);
27+
});
28+
29+
it('reports an offense when the context is valid for products but the input is a cart', async () => {
30+
const sourceCode = `{{ cart | standard_event_data: 'view', context: 'recommendation' }}`;
31+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
32+
33+
expect(offenses).toHaveLength(1);
34+
expect(offenses[0].message).toBe(
35+
"Unsupported context 'recommendation' for cart. Valid values: page, dialog. The 'context' argument can also be omitted.",
36+
);
37+
});
38+
39+
it('does not report an offense on supported product context values', async () => {
40+
const contexts = ['page', 'search', 'collection', 'dialog', 'recommendation'];
41+
42+
for (const context of contexts) {
43+
const sourceCode = `{{ product | standard_event_data: 'view', context: '${context}' }}`;
44+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
45+
46+
expect(offenses, `expected '${context}' to be a valid product context`).toHaveLength(0);
47+
}
48+
});
49+
50+
it('does not report an offense on supported cart context values', async () => {
51+
const contexts = ['page', 'dialog'];
52+
53+
for (const context of contexts) {
54+
const sourceCode = `{{ cart | standard_event_data: 'view', context: '${context}' }}`;
55+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
56+
57+
expect(offenses, `expected '${context}' to be a valid cart context`).toHaveLength(0);
58+
}
59+
});
60+
61+
it('does not report an offense on collections, whose context is ignored', async () => {
62+
const sourceCode = `{{ collection | standard_event_data: 'view', context: 'homepage' }}`;
63+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
64+
65+
expect(offenses).toHaveLength(0);
66+
});
67+
68+
it('falls back to the union of contexts when the input is not a known global', async () => {
69+
const sourceCode = `{{ line_item.product | standard_event_data: 'view', context: 'recommendation' }}`;
70+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
71+
72+
expect(offenses).toHaveLength(0);
73+
});
74+
75+
it('falls back to the union of contexts when the filter input is chained', async () => {
76+
const sourceCode = `{{ cart | default: other_cart | standard_event_data: 'view', context: 'recommendation' }}`;
77+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
78+
79+
expect(offenses).toHaveLength(0);
80+
});
81+
82+
it('reports an offense when the context is outside the union for an unknown input', async () => {
83+
const sourceCode = `{{ line_item.product | standard_event_data: 'view', context: 'homepage' }}`;
84+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
85+
86+
expect(offenses).toHaveLength(1);
87+
expect(offenses[0].message).toBe(
88+
"Unsupported context 'homepage'. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.",
89+
);
90+
});
91+
92+
it('does not report an offense when the context is a variable', async () => {
93+
const sourceCode = `{{ product | standard_event_data: 'view', context: section.settings.context }}`;
94+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
95+
96+
expect(offenses).toHaveLength(0);
97+
});
98+
99+
it('reports an offense when the context is a non-string literal', async () => {
100+
const sourceCode = `{{ product | standard_event_data: 'view', context: 123 }}`;
101+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
102+
103+
expect(offenses).toHaveLength(1);
104+
expect(offenses[0].message).toBe(
105+
"Unsupported context for product. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.",
106+
);
107+
108+
const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses);
109+
expect(highlights[0]).to.eql('123');
110+
});
111+
112+
it('does not report an offense when the context argument is omitted', async () => {
113+
const sourceCode = `{{ collection | standard_event_data: 'view' }}`;
114+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
115+
116+
expect(offenses).toHaveLength(0);
117+
});
118+
119+
it('reports an offense when the event type is not supported', async () => {
120+
const sourceCode = `{{ product | standard_event_data: 'click', context: 'page' }}`;
121+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
122+
123+
expect(offenses).toHaveLength(1);
124+
expect(offenses[0].message).toBe(
125+
"Unsupported event type 'click'. The only supported event type is 'view'.",
126+
);
127+
128+
const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses);
129+
expect(highlights[0]).to.eql("'click'");
130+
});
131+
132+
it('reports an offense when the event type is not supported on a collection', async () => {
133+
const sourceCode = `{{ collection | standard_event_data: 'click' }}`;
134+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
135+
136+
expect(offenses).toHaveLength(1);
137+
expect(offenses[0].message).toBe(
138+
"Unsupported event type 'click'. The only supported event type is 'view'.",
139+
);
140+
});
141+
142+
it('does not report an offense when the event type is a variable', async () => {
143+
const sourceCode = `{{ product | standard_event_data: event_type }}`;
144+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
145+
146+
expect(offenses).toHaveLength(0);
147+
});
148+
149+
it('reports an offense when the event type is a non-string literal', async () => {
150+
const sourceCode = `{{ product | standard_event_data: 123 }}`;
151+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
152+
153+
expect(offenses).toHaveLength(1);
154+
expect(offenses[0].message).toBe(
155+
"Unsupported event type. The only supported event type is 'view'.",
156+
);
157+
158+
const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses);
159+
expect(highlights[0]).to.eql('123');
160+
});
161+
162+
it('reports an offense when the event type is a boolean literal', async () => {
163+
const sourceCode = `{{ product | standard_event_data: true }}`;
164+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
165+
166+
expect(offenses).toHaveLength(1);
167+
expect(offenses[0].message).toBe(
168+
"Unsupported event type. The only supported event type is 'view'.",
169+
);
170+
});
171+
172+
it('reports both offenses when the event type and the context are invalid', async () => {
173+
const sourceCode = `{{ product | standard_event_data: 'click', context: 'homepage' }}`;
174+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
175+
176+
expect(offenses).toHaveLength(2);
177+
});
178+
179+
it('does not report an offense on other filters', async () => {
180+
const sourceCode = `{{ product | json }}{{ 'homepage' | append: 'view' }}`;
181+
const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode);
182+
183+
expect(offenses).toHaveLength(0);
184+
});
185+
});

‎packages/theme-check-common/src/checks/valid-standard-event-data/index.ts‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
import {
2+
LiquidExpression,
3+
LiquidFilter,
4+
LiquidHtmlNode,
5+
LiquidNamedArgument,
6+
NodeTypes,
7+
} from '@shopify/liquid-html-parser';
8+
import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types';
9+
10+
const FILTER_NAME = 'standard_event_data';
11+
const CONTEXT_ARGUMENT = 'context';
12+
const SUPPORTED_EVENT_TYPE = 'view';
13+
14+
const SUPPORTED_CONTEXTS_BY_DROP: { [drop: string]: string[] } = {
15+
product: ['page', 'search', 'collection', 'dialog', 'recommendation'],
16+
cart: ['page', 'dialog'],
17+
};
18+
19+
const DROPS_THAT_IGNORE_CONTEXT = ['collection'];
20+
21+
const CONTEXTS_SUPPORTED_BY_ANY_INPUT_TYPE = [
22+
...new Set(Object.values(SUPPORTED_CONTEXTS_BY_DROP).flat()),
23+
];
24+
25+
function isInvalidStaticValue(value: LiquidExpression, supportedValues: string[]): boolean {
26+
if (value.type === NodeTypes.VariableLookup) return false;
27+
return value.type !== NodeTypes.String || !supportedValues.includes(value.value);
28+
}
29+
30+
function describeValue(value: LiquidExpression): string {
31+
return value.type === NodeTypes.String ? ` '${value.value}'` : '';
32+
}
33+
34+
function detectInputDrop(
35+
node: LiquidFilter,
36+
parent: LiquidHtmlNode | undefined,
37+
): string | undefined {
38+
if (parent?.type !== NodeTypes.LiquidVariable) return undefined;
39+
if (parent.filters[0] !== node) return undefined;
40+
41+
const expression = parent.expression;
42+
if (expression.type !== NodeTypes.VariableLookup) return undefined;
43+
if (expression.lookups.length > 0) return undefined;
44+
45+
return expression.name ?? undefined;
46+
}
47+
48+
export const ValidStandardEventData: LiquidCheckDefinition = {
49+
meta: {
50+
code: 'ValidStandardEventData',
51+
name: 'Prevent the use of invalid arguments to the standard_event_data filter',
52+
docs: {
53+
description:
54+
'This check is aimed at preventing the use of invalid arguments for the standard_event_data filter.',
55+
url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-standard-event-data',
56+
recommended: true,
57+
},
58+
type: SourceCodeType.LiquidHtml,
59+
severity: Severity.ERROR,
60+
schema: {},
61+
targets: [],
62+
},
63+
64+
create(context) {
65+
return {
66+
async LiquidFilter(node, ancestors) {
67+
if (node.name !== FILTER_NAME) return;
68+
69+
const eventType = node.args.find(
70+
(arg): arg is LiquidExpression => arg.type !== NodeTypes.NamedArgument,
71+
);
72+
73+
if (eventType && isInvalidStaticValue(eventType, [SUPPORTED_EVENT_TYPE])) {
74+
context.report({
75+
message: `Unsupported event type${describeValue(
76+
eventType,
77+
)}. The only supported event type is '${SUPPORTED_EVENT_TYPE}'.`,
78+
startIndex: eventType.position.start,
79+
endIndex: eventType.position.end,
80+
});
81+
}
82+
83+
const drop = detectInputDrop(node, ancestors[ancestors.length - 1]);
84+
85+
if (drop && DROPS_THAT_IGNORE_CONTEXT.includes(drop)) return;
86+
87+
const contextArgument = node.args.find(
88+
(arg): arg is LiquidNamedArgument =>
89+
arg.type === NodeTypes.NamedArgument && arg.name === CONTEXT_ARGUMENT,
90+
);
91+
const contextValue = contextArgument?.value;
92+
if (!contextValue) return;
93+
94+
const supportedContexts =
95+
(drop && SUPPORTED_CONTEXTS_BY_DROP[drop]) || CONTEXTS_SUPPORTED_BY_ANY_INPUT_TYPE;
96+
97+
if (isInvalidStaticValue(contextValue, supportedContexts)) {
98+
const dropDescription = drop && SUPPORTED_CONTEXTS_BY_DROP[drop] ? ` for ${drop}` : '';
99+
100+
context.report({
101+
message: `Unsupported context${describeValue(
102+
contextValue,
103+
)}${dropDescription}. Valid values: ${supportedContexts.join(
104+
', ',
105+
)}. The '${CONTEXT_ARGUMENT}' argument can also be omitted.`,
106+
startIndex: contextValue.position.start,
107+
endIndex: contextValue.position.end,
108+
});
109+
}
110+
},
111+
};
112+
},
113+
};

‎packages/theme-check-node/configs/all.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,9 @@ ValidScopedCSSClass:
262262
ValidSettingsKey:
263263
enabled: true
264264
severity: 0
265+
ValidStandardEventData:
266+
enabled: true
267+
severity: 0
265268
ValidStaticBlockType:
266269
enabled: true
267270
severity: 0

‎packages/theme-check-node/configs/recommended.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,9 @@ ValidScopedCSSClass:
240240
ValidSettingsKey:
241241
enabled: true
242242
severity: 0
243+
ValidStandardEventData:
244+
enabled: true
245+
severity: 0
243246
ValidStaticBlockType:
244247
enabled: true
245248
severity: 0

0 commit comments

Comments
 (0)