diff --git a/.changeset/valid-standard-event-data.md b/.changeset/valid-standard-event-data.md new file mode 100644 index 000000000..73fd2435d --- /dev/null +++ b/.changeset/valid-standard-event-data.md @@ -0,0 +1,9 @@ +--- +'@shopify/theme-check-common': minor +--- + +Add `ValidStandardEventData` check to error on invalid arguments to the `standard_event_data` filter. + +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`. + +Values that aren't string literals are left alone, since the type of the piped input isn't statically knowable. diff --git a/packages/theme-check-common/src/checks/index.ts b/packages/theme-check-common/src/checks/index.ts index 74ad5779e..c8db44960 100644 --- a/packages/theme-check-common/src/checks/index.ts +++ b/packages/theme-check-common/src/checks/index.ts @@ -64,6 +64,7 @@ import { ValidSchema } from './valid-schema'; import { ValidSchemaName } from './valid-schema-name'; import { ValidSchemaTranslations } from './valid-schema-translations'; import { ValidSettingsKey } from './valid-settings-key'; +import { ValidStandardEventData } from './valid-standard-event-data'; import { ValidStaticBlockType } from './valid-static-block-type'; import { ValidVisibleIf, ValidVisibleIfSettingsSchema } from './valid-visible-if'; import { VariableName } from './variable-name'; @@ -154,6 +155,7 @@ export const allChecks: (LiquidCheckDefinition | JSONCheckDefinition)[] = [ ValidRenderSnippetArgumentTypes, ValidSchema, ValidSettingsKey, + ValidStandardEventData, ValidStaticBlockType, ValidVisibleIf, ValidVisibleIfSettingsSchema, diff --git a/packages/theme-check-common/src/checks/valid-standard-event-data/index.spec.ts b/packages/theme-check-common/src/checks/valid-standard-event-data/index.spec.ts new file mode 100644 index 000000000..29e87a99c --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-standard-event-data/index.spec.ts @@ -0,0 +1,185 @@ +import { describe, expect, it } from 'vitest'; +import { highlightedOffenses, runLiquidCheck } from '../../test'; +import { ValidStandardEventData } from './index'; + +describe('Module: ValidStandardEventData', () => { + it('reports an offense when the context is not a supported value for products', async () => { + const sourceCode = `{{ product | standard_event_data: 'view', context: 'homepage' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported context 'homepage' for product. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.", + ); + + const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses); + expect(highlights[0]).to.eql("'homepage'"); + }); + + it('reports an offense when the context is not a supported value for carts', async () => { + const sourceCode = `{{ cart | standard_event_data: 'view', context: 'banner' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported context 'banner' for cart. Valid values: page, dialog. The 'context' argument can also be omitted.", + ); + }); + + it('reports an offense when the context is valid for products but the input is a cart', async () => { + const sourceCode = `{{ cart | standard_event_data: 'view', context: 'recommendation' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported context 'recommendation' for cart. Valid values: page, dialog. The 'context' argument can also be omitted.", + ); + }); + + it('does not report an offense on supported product context values', async () => { + const contexts = ['page', 'search', 'collection', 'dialog', 'recommendation']; + + for (const context of contexts) { + const sourceCode = `{{ product | standard_event_data: 'view', context: '${context}' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses, `expected '${context}' to be a valid product context`).toHaveLength(0); + } + }); + + it('does not report an offense on supported cart context values', async () => { + const contexts = ['page', 'dialog']; + + for (const context of contexts) { + const sourceCode = `{{ cart | standard_event_data: 'view', context: '${context}' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses, `expected '${context}' to be a valid cart context`).toHaveLength(0); + } + }); + + it('does not report an offense on collections, whose context is ignored', async () => { + const sourceCode = `{{ collection | standard_event_data: 'view', context: 'homepage' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('falls back to the union of contexts when the input is not a known global', async () => { + const sourceCode = `{{ line_item.product | standard_event_data: 'view', context: 'recommendation' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('falls back to the union of contexts when the filter input is chained', async () => { + const sourceCode = `{{ cart | default: other_cart | standard_event_data: 'view', context: 'recommendation' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('reports an offense when the context is outside the union for an unknown input', async () => { + const sourceCode = `{{ line_item.product | standard_event_data: 'view', context: 'homepage' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported context 'homepage'. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.", + ); + }); + + it('does not report an offense when the context is a variable', async () => { + const sourceCode = `{{ product | standard_event_data: 'view', context: section.settings.context }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('reports an offense when the context is a non-string literal', async () => { + const sourceCode = `{{ product | standard_event_data: 'view', context: 123 }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported context for product. Valid values: page, search, collection, dialog, recommendation. The 'context' argument can also be omitted.", + ); + + const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses); + expect(highlights[0]).to.eql('123'); + }); + + it('does not report an offense when the context argument is omitted', async () => { + const sourceCode = `{{ collection | standard_event_data: 'view' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('reports an offense when the event type is not supported', async () => { + const sourceCode = `{{ product | standard_event_data: 'click', context: 'page' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported event type 'click'. The only supported event type is 'view'.", + ); + + const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses); + expect(highlights[0]).to.eql("'click'"); + }); + + it('reports an offense when the event type is not supported on a collection', async () => { + const sourceCode = `{{ collection | standard_event_data: 'click' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported event type 'click'. The only supported event type is 'view'.", + ); + }); + + it('does not report an offense when the event type is a variable', async () => { + const sourceCode = `{{ product | standard_event_data: event_type }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); + + it('reports an offense when the event type is a non-string literal', async () => { + const sourceCode = `{{ product | standard_event_data: 123 }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported event type. The only supported event type is 'view'.", + ); + + const highlights = highlightedOffenses({ 'file.liquid': sourceCode }, offenses); + expect(highlights[0]).to.eql('123'); + }); + + it('reports an offense when the event type is a boolean literal', async () => { + const sourceCode = `{{ product | standard_event_data: true }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(1); + expect(offenses[0].message).toBe( + "Unsupported event type. The only supported event type is 'view'.", + ); + }); + + it('reports both offenses when the event type and the context are invalid', async () => { + const sourceCode = `{{ product | standard_event_data: 'click', context: 'homepage' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(2); + }); + + it('does not report an offense on other filters', async () => { + const sourceCode = `{{ product | json }}{{ 'homepage' | append: 'view' }}`; + const offenses = await runLiquidCheck(ValidStandardEventData, sourceCode); + + expect(offenses).toHaveLength(0); + }); +}); diff --git a/packages/theme-check-common/src/checks/valid-standard-event-data/index.ts b/packages/theme-check-common/src/checks/valid-standard-event-data/index.ts new file mode 100644 index 000000000..ffbe61a66 --- /dev/null +++ b/packages/theme-check-common/src/checks/valid-standard-event-data/index.ts @@ -0,0 +1,113 @@ +import { + LiquidExpression, + LiquidFilter, + LiquidHtmlNode, + LiquidNamedArgument, + NodeTypes, +} from '@shopify/liquid-html-parser'; +import { LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; + +const FILTER_NAME = 'standard_event_data'; +const CONTEXT_ARGUMENT = 'context'; +const SUPPORTED_EVENT_TYPE = 'view'; + +const SUPPORTED_CONTEXTS_BY_DROP: { [drop: string]: string[] } = { + product: ['page', 'search', 'collection', 'dialog', 'recommendation'], + cart: ['page', 'dialog'], +}; + +const DROPS_THAT_IGNORE_CONTEXT = ['collection']; + +const CONTEXTS_SUPPORTED_BY_ANY_INPUT_TYPE = [ + ...new Set(Object.values(SUPPORTED_CONTEXTS_BY_DROP).flat()), +]; + +function isInvalidStaticValue(value: LiquidExpression, supportedValues: string[]): boolean { + if (value.type === NodeTypes.VariableLookup) return false; + return value.type !== NodeTypes.String || !supportedValues.includes(value.value); +} + +function describeValue(value: LiquidExpression): string { + return value.type === NodeTypes.String ? ` '${value.value}'` : ''; +} + +function detectInputDrop( + node: LiquidFilter, + parent: LiquidHtmlNode | undefined, +): string | undefined { + if (parent?.type !== NodeTypes.LiquidVariable) return undefined; + if (parent.filters[0] !== node) return undefined; + + const expression = parent.expression; + if (expression.type !== NodeTypes.VariableLookup) return undefined; + if (expression.lookups.length > 0) return undefined; + + return expression.name ?? undefined; +} + +export const ValidStandardEventData: LiquidCheckDefinition = { + meta: { + code: 'ValidStandardEventData', + name: 'Prevent the use of invalid arguments to the standard_event_data filter', + docs: { + description: + 'This check is aimed at preventing the use of invalid arguments for the standard_event_data filter.', + url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/valid-standard-event-data', + recommended: true, + }, + type: SourceCodeType.LiquidHtml, + severity: Severity.ERROR, + schema: {}, + targets: [], + }, + + create(context) { + return { + async LiquidFilter(node, ancestors) { + if (node.name !== FILTER_NAME) return; + + const eventType = node.args.find( + (arg): arg is LiquidExpression => arg.type !== NodeTypes.NamedArgument, + ); + + if (eventType && isInvalidStaticValue(eventType, [SUPPORTED_EVENT_TYPE])) { + context.report({ + message: `Unsupported event type${describeValue( + eventType, + )}. The only supported event type is '${SUPPORTED_EVENT_TYPE}'.`, + startIndex: eventType.position.start, + endIndex: eventType.position.end, + }); + } + + const drop = detectInputDrop(node, ancestors[ancestors.length - 1]); + + if (drop && DROPS_THAT_IGNORE_CONTEXT.includes(drop)) return; + + const contextArgument = node.args.find( + (arg): arg is LiquidNamedArgument => + arg.type === NodeTypes.NamedArgument && arg.name === CONTEXT_ARGUMENT, + ); + const contextValue = contextArgument?.value; + if (!contextValue) return; + + const supportedContexts = + (drop && SUPPORTED_CONTEXTS_BY_DROP[drop]) || CONTEXTS_SUPPORTED_BY_ANY_INPUT_TYPE; + + if (isInvalidStaticValue(contextValue, supportedContexts)) { + const dropDescription = drop && SUPPORTED_CONTEXTS_BY_DROP[drop] ? ` for ${drop}` : ''; + + context.report({ + message: `Unsupported context${describeValue( + contextValue, + )}${dropDescription}. Valid values: ${supportedContexts.join( + ', ', + )}. The '${CONTEXT_ARGUMENT}' argument can also be omitted.`, + startIndex: contextValue.position.start, + endIndex: contextValue.position.end, + }); + } + }, + }; + }, +}; diff --git a/packages/theme-check-node/configs/all.yml b/packages/theme-check-node/configs/all.yml index c80341be9..1ae1e8ff0 100644 --- a/packages/theme-check-node/configs/all.yml +++ b/packages/theme-check-node/configs/all.yml @@ -262,6 +262,9 @@ ValidScopedCSSClass: ValidSettingsKey: enabled: true severity: 0 +ValidStandardEventData: + enabled: true + severity: 0 ValidStaticBlockType: enabled: true severity: 0 diff --git a/packages/theme-check-node/configs/recommended.yml b/packages/theme-check-node/configs/recommended.yml index 065a10050..0f687f95b 100644 --- a/packages/theme-check-node/configs/recommended.yml +++ b/packages/theme-check-node/configs/recommended.yml @@ -240,6 +240,9 @@ ValidScopedCSSClass: ValidSettingsKey: enabled: true severity: 0 +ValidStandardEventData: + enabled: true + severity: 0 ValidStaticBlockType: enabled: true severity: 0