Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add deeplinking for block types #674

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,45 @@ describe('DocumentLinksProvider', () => {
expect(result[i].target).toBe(expectedUrls[i]);
}
});

it('should return a list of document links for customizable theme block types', async () => {
// @theme and @app are not included as these are general types used for block targeting
// inline blocks are not included as there are no related block files to link to
uriString = 'file:///path/to/liquid-raw-tag-document.liquid';
rootUri = 'file:///path/to/project';

const liquidRawTagContent = `
{% schema %}
{ "blocks": [{ "type": "@theme" }, {"type": "top_level" }, {"type": "_private_block" }],
"name": "Test section",
"presets": [
{
"blocks": {
"block-1": {
"name": "inline block",
"type": "inline_block"
},
"block-2": {
"type": "nested_block"
}
}
}
]
}
{% endschema %}
`;

documentManager.open(uriString, liquidRawTagContent, 1);

const result = await documentLinksProvider.documentLinks(uriString);
const expectedUrls = [
'file:///path/to/project/blocks/top_level.liquid',
'file:///path/to/project/blocks/_private_block.liquid',
'file:///path/to/project/blocks/nested_block.liquid',
];
expect(result.length).toBe(expectedUrls.length);
for (let i = 0; i < expectedUrls.length; i++) {
expect(result[i].target).toBe(expectedUrls[i]);
}
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { LiquidHtmlNode, LiquidString, NodeTypes } from '@shopify/liquid-html-parser';
import { LiquidHtmlNode, LiquidRawTag, LiquidString, NodeTypes } from '@shopify/liquid-html-parser';
import { SourceCodeType } from '@shopify/theme-check-common';
import { DocumentLink, Range } from 'vscode-languageserver';
import { TextDocument } from 'vscode-languageserver-textdocument';
Expand All @@ -7,6 +7,8 @@ import { URI, Utils } from 'vscode-uri';
import { DocumentManager } from '../documents';
import { visit, Visitor } from '@shopify/theme-check-common';

import { parseTree, findNodeAtLocation, ParseError, Node as JSONNode } from 'jsonc-parser';

export class DocumentLinksProvider {
constructor(
private documentManager: DocumentManager,
Expand Down Expand Up @@ -79,6 +81,36 @@ function documentLinksVisitor(
Utils.resolvePath(root, 'assets', expression.value).toString(),
);
},

LiquidRawTag(node) {
if (node.name === 'schema') {
const errors: ParseError[] = [];
const jsonNode = parseTree(node.body.value, errors);
if (!jsonNode || errors.length > 0) {
return [];
}

const links: DocumentLink[] = [];

const blocksNode = findNodeAtLocation(jsonNode, ['blocks']);
if (blocksNode && blocksNode.type === 'array' && blocksNode.children) {
links.push(...createLinksFromBlocks(blocksNode, node, textDocument, root));
}

const presetsNode = findNodeAtLocation(jsonNode, ['presets']);
if (presetsNode && presetsNode.type === 'array' && presetsNode.children) {
presetsNode.children.forEach((presetNode) => {
const presetBlocksNode = findNodeAtLocation(presetNode, ['blocks']);
if (presetBlocksNode) {
links.push(...processPresetBlocks(presetBlocksNode, node, textDocument, root));
}
});
}

return links;
}
return [];
},
};
}

Expand All @@ -91,3 +123,144 @@ function range(textDocument: TextDocument, node: { position: LiquidHtmlNode['pos
function isLiquidString(node: LiquidHtmlNode): node is LiquidString {
return node.type === NodeTypes.String;
}

function createDocumentLinkForTypeNode(
typeNode: JSONNode,
parentNode: LiquidRawTag,
textDocument: TextDocument,
root: URI,
blockType: string,
): DocumentLink | null {
const startOffset = typeNode.offset;
const endOffset = typeNode.offset + typeNode.length;
const startPos = parentNode.body.position.start + startOffset;
const endPos = parentNode.body.position.start + endOffset;

const start = textDocument.positionAt(startPos);
const end = textDocument.positionAt(endPos);

return DocumentLink.create(
Range.create(start, end),
Utils.resolvePath(root, 'blocks', `${blockType}.liquid`).toString(),
);
}

function processPresetBlocks(
blocksNode: JSONNode,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of a hefty function. it would help with scanability to encapsulate common functionality between the conditional branches.

parentNode: LiquidRawTag,
textDocument: TextDocument,
root: URI,
): DocumentLink[] {
const links: DocumentLink[] = [];

if (blocksNode.type === 'object' && blocksNode.children) {
blocksNode.children.forEach((propertyNode) => {
const blockValueNode = propertyNode.children?.[1];
if (!blockValueNode) return;

const nameNode = findNodeAtLocation(blockValueNode, ['name']);
if (nameNode) {
return;
}

const typeNode = findNodeAtLocation(blockValueNode, ['type']);
if (typeNode && typeNode.type === 'string' && typeof typeNode.value === 'string') {
const blockType = typeNode.value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When evaluating a generic variable conforms to a specific structure, you can do something called "type narrowing" when there is a specific type definition for the structure of data that you're verifying for. This has typescript benefits of showing proper typing feedback as you write code inside these conditionals. Take a look at these examples of type narrowing code in the theme-tools repo: https://github.com/Shopify/theme-tools/blob/deeplink-theme-blocks/packages/theme-check-common/src/types.ts#L23-L25

if (blockType.startsWith('@')) {
return;
}

const link = createDocumentLinkForTypeNode(
typeNode,
parentNode,
textDocument,
root,
blockType,
);

if (link) {
links.push(link);
}
}

const nestedBlocksNode = findNodeAtLocation(blockValueNode, ['blocks']);
if (nestedBlocksNode) {
links.push(...processPresetBlocks(nestedBlocksNode, parentNode, textDocument, root));
}
});
} else if (blocksNode.type === 'array' && blocksNode.children) {
blocksNode.children.forEach((blockNode) => {
const nameNode = findNodeAtLocation(blockNode, ['name']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One way to reduce this big function is to move the iteree code within the forEach into a standalone function definition.

if (nameNode) {
return;
}

const typeNode = findNodeAtLocation(blockNode, ['type']);
if (typeNode && typeNode.type === 'string' && typeof typeNode.value === 'string') {
const blockType = typeNode.value;
if (blockType.startsWith('@')) {
return;
}

const link = createDocumentLinkForTypeNode(
typeNode,
parentNode,
textDocument,
root,
blockType,
);

if (link) {
links.push(link);
}
}

const nestedBlocksNode = findNodeAtLocation(blockNode, ['blocks']);
if (nestedBlocksNode) {
links.push(...processPresetBlocks(nestedBlocksNode, parentNode, textDocument, root));
}
});
}

return links;
}

function createLinksFromBlocks(
blocksNode: JSONNode,
parentNode: LiquidRawTag,
textDocument: TextDocument,
root: URI,
): DocumentLink[] {
const links: DocumentLink[] = [];

if (blocksNode.children) {
blocksNode.children.forEach((blockNode: JSONNode) => {
const nameNode = findNodeAtLocation(blockNode, ['name']);
if (nameNode) {
return;
}

const typeNode = findNodeAtLocation(blockNode, ['type']);
if (typeNode && typeNode.type === 'string' && typeof typeNode.value === 'string') {
const blockType = typeNode.value;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same comment about type narrowing this to something more specific from JSONNode

if (blockType.startsWith('@')) {
return;
}

const link = createDocumentLinkForTypeNode(
typeNode,
parentNode,
textDocument,
root,
blockType,
);

if (link) {
links.push(link);
}
}
});
}

return links;
}
Loading