Skip to content
Open
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 @@ -327,6 +327,35 @@ describe('Module: UndefinedObject', () => {
}
});

it('should allow block-level objects in snippets when in app mode', async () => {
const blockLevelObjects = ['section', 'block', 'recommendations', 'app'];
for (const object of blockLevelObjects) {
const sourceCode = `{% doc %} @param {string} x - x {% enddoc %}{{ ${object} }}`;
const offenses = await runLiquidCheck(
UndefinedObject,
sourceCode,
'snippets/my-snippet.liquid',
{},
undefined,
'app',
);
expect(offenses, `Expected no offense for '${object}' in app mode snippet`).toHaveLength(0);
}
});

it('should still flag block-level objects in snippets when in theme mode', async () => {
const blockOnlyObjects = ['section', 'block', 'recommendations'];
for (const object of blockOnlyObjects) {
const sourceCode = `{% doc %} @param {string} x - x {% enddoc %}{{ ${object} }}`;
const offenses = await runLiquidCheck(
UndefinedObject,
sourceCode,
'snippets/my-snippet.liquid',
);
expect(offenses, `Expected offense for '${object}' in theme mode snippet`).toHaveLength(1);
}
});

it('should support contextual exceptions for checkout.liquid', async () => {
let offenses: Offense[];
const contexts: [string, string][] = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import {
NodeTypes,
Position,
} from '@shopify/liquid-html-parser';
import { LiquidCheckDefinition, Severity, SourceCodeType, ThemeDocset } from '../../types';
import { LiquidCheckDefinition, Mode, Severity, SourceCodeType, ThemeDocset } from '../../types';
import { isError, last } from '../../utils';
import { hasLiquidDoc } from '../../liquid-doc/liquidDoc';
import { isWithinRawTagThatDoesNotParseItsContents } from '../utils';
Expand Down Expand Up @@ -146,7 +146,7 @@ export const UndefinedObject: LiquidCheckDefinition = {
},

async onCodePathEnd() {
const objects = await globalObjects(themeDocset, relativePath);
const objects = await globalObjects(themeDocset, relativePath, context.mode);

objects.forEach((obj) => fileScopedVariables.add(obj.name));

Expand All @@ -172,9 +172,9 @@ export const UndefinedObject: LiquidCheckDefinition = {
},
};

async function globalObjects(themeDocset: ThemeDocset, relativePath: string) {
async function globalObjects(themeDocset: ThemeDocset, relativePath: string, mode: Mode = 'theme') {
const objects = await themeDocset.objects();
const contextualObjects = getContextualObjects(relativePath);
const contextualObjects = getContextualObjects(relativePath, mode);

const globalObjects = objects.filter(({ access, name }) => {
return (
Expand All @@ -188,7 +188,9 @@ async function globalObjects(themeDocset: ThemeDocset, relativePath: string) {
return globalObjects;
}

function getContextualObjects(relativePath: string): string[] {
const BLOCK_CONTEXTUAL_OBJECTS = ['app', 'section', 'recommendations', 'block'];

function getContextualObjects(relativePath: string, mode: Mode = 'theme'): string[] {
if (relativePath.startsWith('layout/checkout.liquid')) {
return [
'locale',
Expand All @@ -211,10 +213,13 @@ function getContextualObjects(relativePath: string): string[] {
}

if (relativePath.startsWith('blocks/')) {
return ['app', 'section', 'recommendations', 'block'];
return BLOCK_CONTEXTUAL_OBJECTS;
}

if (relativePath.startsWith('snippets/')) {
// In a theme app extension, snippets can only be rendered from blocks,
// so they have access to the same contextual objects as blocks.
if (mode === 'app') return BLOCK_CONTEXTUAL_OBJECTS;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of threading mode all the way through we could do something really lazy here but the cost doesn't seem too bad and it sets up a pattern that we might want to use in other areas.

return ['app'];
}

Expand Down
7 changes: 6 additions & 1 deletion packages/theme-check-common/src/test/test-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
JSONCorrector,
JSONSourceCode,
LiquidSourceCode,
Mode,
Offense,
recommended,
SectionSchema,
Expand Down Expand Up @@ -44,10 +45,11 @@ export async function check(
checks: CheckDefinition[] = recommended,
mockDependencies: Partial<Dependencies> = {},
checkSettings: ChecksSettings = {},
mode: Mode = 'theme',
): Promise<Offense[]> {
const theme = getTheme(themeDesc);
const config: Config = {
context: 'theme',
context: mode,
settings: { ...checkSettings },
checks,
rootUri,
Expand Down Expand Up @@ -236,11 +238,14 @@ export async function runLiquidCheck(
fileName: string = 'file.liquid',
mockDependencies: Partial<Dependencies> = {},
existingThemeFiles?: MockTheme,
mode: Mode = 'theme',
): Promise<Offense[]> {
const offenses = await check(
{ ...existingThemeFiles, [fileName]: sourceCode },
[checkDef],
mockDependencies,
{},
mode,
);
return offenses.filter((offense) => offense.uri === path.join(rootUri, fileName));
}
Expand Down
17 changes: 13 additions & 4 deletions packages/theme-language-server-common/src/TypeSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
FilterEntry,
MetafieldDefinitionMap,
MetafieldDefinition,
Mode,
ObjectEntry,
ReturnType,
SourceCodeType,
Expand All @@ -39,11 +40,14 @@ import {
import { findLast, memo } from './utils';
import { visit } from '@shopify/theme-check-common';

export type GetModeForURI = (uri: string) => Promise<Mode>;

export class TypeSystem {
constructor(
private readonly themeDocset: ThemeDocset,
private readonly getThemeSettingsSchemaForURI: GetThemeSettingsSchemaForURI,
private readonly getMetafieldDefinitions: (rootUri: string) => Promise<MetafieldDefinitionMap>,
private readonly getModeForURI: GetModeForURI = async () => 'theme',
) {}

async inferType(
Expand Down Expand Up @@ -321,8 +325,8 @@ export class TypeSystem {
});

private contextualVariables = async (uri: string) => {
const entries = await this.objectEntries();
const contextualEntries = getContextualEntries(uri);
const [entries, mode] = await Promise.all([this.objectEntries(), this.getModeForURI(uri)]);
const contextualEntries = getContextualEntries(uri, mode);
return entries.filter((entry) => contextualEntries.includes(entry.name));
};
}
Expand All @@ -332,7 +336,9 @@ const BLOCK_FILE_REGEX = /blocks[\/\\][^.\\\/]*\.liquid$/;
const SNIPPET_FILE_REGEX = /snippets[\/\\][^.\\\/]*\.liquid$/;
const LAYOUT_FILE_REGEX = /layout[\/\\]checkout\.liquid$/;

function getContextualEntries(uri: string): string[] {
const BLOCK_CONTEXTUAL_ENTRIES = ['app', 'section', 'recommendations', 'block'];

function getContextualEntries(uri: string, mode: Mode = 'theme'): string[] {
const normalizedUri = path.normalize(uri);
if (LAYOUT_FILE_REGEX.test(normalizedUri)) {
return [
Expand All @@ -355,9 +361,12 @@ function getContextualEntries(uri: string): string[] {
return ['section', 'predictive_search', 'recommendations', 'comment'];
}
if (BLOCK_FILE_REGEX.test(normalizedUri)) {
return ['app', 'section', 'recommendations', 'block'];
return BLOCK_CONTEXTUAL_ENTRIES;
}
if (SNIPPET_FILE_REGEX.test(normalizedUri)) {
// In a theme app extension, snippets can only be rendered from blocks,
// so they have access to the same contextual objects as blocks.
if (mode === 'app') return BLOCK_CONTEXTUAL_ENTRIES;
return ['app'];
}
return [];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
GetDocDefinitionForURI,
MetafieldDefinitionMap,
Mode,
SourceCodeType,
ThemeDocset,
} from '@shopify/theme-check-common';
Expand Down Expand Up @@ -40,6 +41,7 @@ export interface CompletionProviderDependencies {
getMetafieldDefinitions: (rootUri: string) => Promise<MetafieldDefinitionMap>;
getDocDefinitionForURI?: GetDocDefinitionForURI;
getThemeBlockNames?: (rootUri: string, includePrivate: boolean) => Promise<string[]>;
getModeForURI?: (uri: string) => Promise<Mode>;
log?: (message: string) => void;
}

Expand All @@ -58,6 +60,7 @@ export class CompletionsProvider {
getThemeSettingsSchemaForURI = async () => [],
getDocDefinitionForURI = async (uri, _relativePath) => ({ uri }),
getThemeBlockNames = async (_rootUri: string, _includePrivate: boolean) => [],
getModeForURI,
log = () => {},
}: CompletionProviderDependencies) {
this.documentManager = documentManager;
Expand All @@ -67,6 +70,7 @@ export class CompletionsProvider {
themeDocset,
getThemeSettingsSchemaForURI,
getMetafieldDefinitions,
getModeForURI,
);

this.providers = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,14 @@ describe('Module: ObjectCompletionProvider', async () => {
parents: [],
},
},
{
name: 'app',
access: {
global: false,
template: [],
parents: [],
},
},
{
name: 'product',
properties: [
Expand Down Expand Up @@ -216,6 +224,46 @@ describe('Module: ObjectCompletionProvider', async () => {
}
});

it('should complete block-level contextual variables in snippets when in app mode', async () => {
const appModeProvider = new CompletionsProvider({
documentManager: new DocumentManager(),
themeDocset: provider.themeDocset,
getMetafieldDefinitions: async () =>
({
article: [],
blog: [],
collection: [],
company: [],
company_location: [],
location: [],
market: [],
order: [],
page: [],
product: [],
variant: [],
shop: [],
}) as MetafieldDefinitionMap,
getModeForURI: async () => 'app',
});

const blockLevelObjects = ['section', 'block', 'recommendations', 'app'];
for (const object of blockLevelObjects) {
const source = `{{ ${object}█ }}`;
await expect(appModeProvider, source).to.complete(
{ source, relativePath: 'snippets/my-snippet.liquid' },
expect.arrayContaining([expect.objectContaining({ label: object })]),
);
}
});

it('should not complete block-level contextual variables in snippets when in theme mode', async () => {
const source = `{{ section█ }}`;
await expect(provider, source).to.complete(
{ source, relativePath: 'snippets/my-snippet.liquid' },
[],
);
});

it('should not complete anything if there is nothing to complete', async () => {
await expect(provider).to.complete('{% assign x = "█" %}', []);
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
GetDocDefinitionForURI,
MetafieldDefinitionMap,
Mode,
SourceCodeType,
ThemeDocset,
} from '@shopify/theme-check-common';
Expand Down Expand Up @@ -37,11 +38,13 @@ export class HoverProvider {
readonly getTranslationsForURI: GetTranslationsForURI = async () => ({}),
readonly getSettingsSchemaForURI: GetThemeSettingsSchemaForURI = async () => [],
readonly getDocDefinitionForURI: GetDocDefinitionForURI = async () => undefined,
readonly getModeForURI: (uri: string) => Promise<Mode> = async () => 'theme',
) {
const typeSystem = new TypeSystem(
themeDocset,
getSettingsSchemaForURI,
getMetafieldDefinitions,
getModeForURI,
);
this.providers = [
new ContentForArgumentHoverProvider(getDocDefinitionForURI),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ export function startServer(
getThemeBlockNames,
getMetafieldDefinitions,
getDocDefinitionForURI,
getModeForURI,
});
const hoverProvider = new HoverProvider(
documentManager,
Expand All @@ -347,6 +348,7 @@ export function startServer(
getTranslationsForURI,
getThemeSettingsSchemaForURI,
getDocDefinitionForURI,
getModeForURI,
);

const executeCommandProvider = new ExecuteCommandProvider(
Expand Down
Loading