-
Notifications
You must be signed in to change notification settings - Fork 37
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Merge pull request #659 from Shopify/miaz/check-preset-static-blocks
static blocks content_for check
- Loading branch information
Showing
7 changed files
with
288 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| --- | ||
| '@shopify/theme-check-common': minor | ||
| '@shopify/theme-check-node': minor | ||
| 'theme-check-vscode': minor | ||
| --- | ||
|
|
||
| Add `SchemaPresetsStaticBlocks` check |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
145 changes: 145 additions & 0 deletions
145
packages/theme-check-common/src/checks/schema-presets-static-blocks/index.spec.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,145 @@ | ||
| import { expect, describe, it } from 'vitest'; | ||
| import { highlightedOffenses, runLiquidCheck, check } from '../../test'; | ||
| import { SchemaPresetsStaticBlocks } from './index'; | ||
|
|
||
| const DEFAULT_FILE_NAME = 'sections/file.liquid'; | ||
|
|
||
| describe('Module: SchemaPresetsStaticBlocks', () => { | ||
| it('reports no errors when there are {% content_for "block" ... %} for each static block in the preset blocks array', async () => { | ||
| const sourceCode = ` | ||
| {% content_for "block" type:"text" id: "block-1" %} | ||
| {% content_for "block" type:"icon" id: "block-2" %} | ||
| {% schema %} | ||
| { | ||
| "name": "Test section", | ||
| "blocks": [{"type": "@theme"}], | ||
| "presets": [ | ||
| { | ||
| "name": "Preset with two static blocks", | ||
| "blocks": [ | ||
| { | ||
| "type": "text", | ||
| "static": true, | ||
| "id": "block-1" | ||
| }, | ||
| { | ||
| "type": "icon", | ||
| "static": true, | ||
| "id": "block-2" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| {% endschema %}`; | ||
|
|
||
| const offenses = await runLiquidCheck(SchemaPresetsStaticBlocks, sourceCode, DEFAULT_FILE_NAME); | ||
| expect(offenses).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('reports an error when there are {% content_for "block" ... %} missing for static blocks in the preset blocks array', async () => { | ||
| const sourceCode = ` | ||
| {% content_for "block" type:"text" id:"block-1" %} | ||
| {% comment %} here we are missing the other content_for block for block-2 {% endcomment %} | ||
| {% schema %} | ||
| { | ||
| "name": "Test section", | ||
| "blocks": [{"type": "@theme"}], | ||
| "presets": [ | ||
| { | ||
| "name": "Preset with two static blocks", | ||
| "blocks": [ | ||
| { | ||
| "type": "text", | ||
| "static": true, | ||
| "id": "block-1" | ||
| }, | ||
| { | ||
| "type": "icon", | ||
| "static": true, | ||
| "id": "block-2" | ||
| } | ||
| ] | ||
| } | ||
| ] | ||
| } | ||
| {% endschema %}`; | ||
|
|
||
| const offenses = await runLiquidCheck(SchemaPresetsStaticBlocks, sourceCode, DEFAULT_FILE_NAME); | ||
| expect(offenses).toHaveLength(1); | ||
| console.log(offenses); | ||
| expect(offenses[0].message).toEqual( | ||
| 'Static block block-2 is missing a corresponding content_for "block" tag.', | ||
| ); | ||
|
|
||
| const highlights = highlightedOffenses({ [DEFAULT_FILE_NAME]: sourceCode }, offenses); | ||
| expect(highlights).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('reports no errors when there are {% content_for "block" ... %} for each static block in the preset blocks hash', async () => { | ||
| const sourceCode = ` | ||
| {% content_for "block" type:"text" id: "block-1" %} | ||
| {% content_for "block" type:"icon" id: "block-2" %} | ||
| {% schema %} | ||
| { | ||
| "name": "Test section", | ||
| "blocks": [{"type": "@theme"}], | ||
| "presets": [ | ||
| { | ||
| "name": "Preset with two static blocks", | ||
| "blocks": { | ||
| "block-1": { | ||
| "type": "text", | ||
| "static": true | ||
| }, | ||
| "block-2": { | ||
| "type": "icon", | ||
| "static": true | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| {% endschema %}`; | ||
|
|
||
| const offenses = await runLiquidCheck(SchemaPresetsStaticBlocks, sourceCode, DEFAULT_FILE_NAME); | ||
| expect(offenses).toHaveLength(0); | ||
| }); | ||
|
|
||
| it('reports an error when there are {% content_for "block" ... %} missing for static blocks in the preset blocks hash', async () => { | ||
| const sourceCode = ` | ||
| {% content_for "block" type:"text" id:"block-1" %} | ||
| {% comment %} here we are missing the other content_for block for block-2 {% endcomment %} | ||
| {% schema %} | ||
| { | ||
| "name": "Test section", | ||
| "blocks": [{"type": "@theme"}], | ||
| "presets": [ | ||
| { | ||
| "name": "Preset with two static blocks", | ||
| "blocks": { | ||
| "block-1": { | ||
| "type": "text", | ||
| "static": true | ||
| }, | ||
| "block-2": { | ||
| "type": "icon", | ||
| "static": true | ||
| } | ||
| } | ||
| } | ||
| ] | ||
| } | ||
| {% endschema %}`; | ||
|
|
||
| const offenses = await runLiquidCheck(SchemaPresetsStaticBlocks, sourceCode, DEFAULT_FILE_NAME); | ||
| expect(offenses).toHaveLength(1); | ||
| console.log(offenses); | ||
| expect(offenses[0].message).toEqual( | ||
| 'Static block block-2 is missing a corresponding content_for "block" tag.', | ||
| ); | ||
|
|
||
| const highlights = highlightedOffenses({ [DEFAULT_FILE_NAME]: sourceCode }, offenses); | ||
| expect(highlights).toHaveLength(1); | ||
| }); | ||
| }); |
126 changes: 126 additions & 0 deletions
126
packages/theme-check-common/src/checks/schema-presets-static-blocks/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,126 @@ | ||
| import { NamedTags, NodeTypes } from '@shopify/liquid-html-parser'; | ||
| import { getLocEnd, getLocStart, nodeAtPath } from '../../json'; | ||
| import { getSchema } from '../../to-schema'; | ||
| import { ArrayNode, LiquidCheckDefinition, Severity, SourceCodeType } from '../../types'; | ||
| import { isContentForBlock } from '../../utils/markup'; | ||
|
|
||
| export const SchemaPresetsStaticBlocks: LiquidCheckDefinition = { | ||
| meta: { | ||
| code: 'SchemaPresetsStaticBlocks', | ||
| name: 'Ensure the preset static blocks are used in the liquid', | ||
| docs: { | ||
| description: | ||
| 'Warns if a preset static block does not have a corresponding content_for "block" tag.', | ||
| recommended: true, | ||
| url: 'https://shopify.dev/docs/storefronts/themes/tools/theme-check/checks/schema-presets-static-blocks', | ||
| }, | ||
| type: SourceCodeType.LiquidHtml, | ||
| severity: Severity.ERROR, | ||
| schema: {}, | ||
| targets: [], | ||
| }, | ||
|
|
||
| create(context) { | ||
| type contentForBlock = { | ||
| id: string; | ||
| type: string; | ||
| }; | ||
| type StaticBlock = { | ||
| id: string; | ||
| type: string; | ||
| startIndex: number; | ||
| endIndex: number; | ||
| }; | ||
| let contentForBlockList: contentForBlock[] = []; | ||
| let staticBlockList: StaticBlock[] = []; | ||
| let offset: number = 0; | ||
|
|
||
| function checkStaticBlocks() { | ||
| staticBlockList.forEach((block) => { | ||
| if ( | ||
| !contentForBlockList.some( | ||
| (contentBlock) => contentBlock.id === block.id && contentBlock.type === block.type, | ||
| ) | ||
| ) { | ||
| context.report({ | ||
| message: `Static block ${block.id} is missing a corresponding content_for "block" tag.`, | ||
| startIndex: block.startIndex, | ||
| endIndex: block.endIndex, | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return { | ||
| async LiquidTag(node) { | ||
| // Early return if not a content_for block tag | ||
| if (node.name !== NamedTags.content_for || !isContentForBlock(node.markup)) return; | ||
|
|
||
| // Extract id and type from markup args | ||
| const idValue = node.markup.args.find((arg) => arg.name === 'id')?.value; | ||
| const typeArg = node.markup.args.find((arg) => arg.name === 'type')?.value; | ||
| if (!typeArg || typeArg.type !== NodeTypes.String) { | ||
| return; // covered by VariableContentForArguments | ||
| } | ||
| const typeValue = typeArg.value; | ||
|
|
||
| // Add to list if valid string id | ||
| if (idValue?.type === NodeTypes.String) { | ||
| contentForBlockList.push({ id: idValue.value, type: typeValue }); | ||
| } | ||
| }, | ||
|
|
||
| async LiquidRawTag(node) { | ||
| // when we get the schema tag, get the list of static blocks from each preset | ||
| if (node.name === 'schema' && node.body.kind === 'json') { | ||
| offset = node.blockStartPosition.end; | ||
| const schema = await getSchema(context); | ||
| const { validSchema, ast } = schema ?? {}; | ||
| if (!validSchema || validSchema instanceof Error) return; | ||
| if (!ast || ast instanceof Error) return; | ||
|
|
||
| const presets = validSchema.presets; | ||
| if (!presets) return; | ||
|
|
||
| presets.forEach((preset, index) => { | ||
| if ('blocks' in preset && preset.blocks) { | ||
| let ast_path: any[] = ['presets', index, 'blocks']; | ||
| // blocks as an array | ||
| if (Array.isArray(preset.blocks)) { | ||
| preset.blocks.forEach((block, block_index) => { | ||
| if (block.static === true && block.id) { | ||
| let node = nodeAtPath(ast, ast_path.concat([block_index]))! as ArrayNode; | ||
| staticBlockList.push({ | ||
| id: block.id, | ||
| type: block.type, | ||
| startIndex: offset + getLocStart(node), | ||
| endIndex: offset + getLocEnd(node), | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| // blocks as an object | ||
| else if (typeof preset.blocks === 'object') { | ||
| Object.entries(preset.blocks).forEach(([block_id, block]) => { | ||
| if (block.static === true) { | ||
| let node = nodeAtPath(ast, ast_path.concat(block_id))! as ArrayNode; | ||
| staticBlockList.push({ | ||
| id: block_id, | ||
| type: block.type, | ||
| startIndex: offset + getLocStart(node), | ||
| endIndex: offset + getLocEnd(node), | ||
| }); | ||
| } | ||
| }); | ||
| } | ||
| } | ||
| }); | ||
| } | ||
| }, | ||
|
|
||
| async onCodePathEnd() { | ||
| checkStaticBlocks(); | ||
| }, | ||
| }; | ||
| }, | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters