Skip to content

Commit 449d162

Browse files
committed
Fix bug where theme check would fail for theme app extensions
This bug only surfaces when a developer would manually run the theme check against a theme app extension from the CLI. Inside the VSCode extension we were already doing the right thing so you wouldn't notice it there and the Shopify CLI hardcodes the check when you build your app so you wouldn't notice it there either. This adds some logic to where we load the config to check if we're inside of an app by looking for two app-like config files. If so, we use `theme-check:theme-app-extension` as our default checks rather than the recommended. This ensures that our context is appropriately set as 'app' instead of 'theme' and removes false positives when running the check.
1 parent ea9f245 commit 449d162

File tree

3 files changed

+43
-2
lines changed

3 files changed

+43
-2
lines changed

.changeset/slow-bananas-sing.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme-check-node': patch
3+
---
4+
5+
Fix false positives when running theme check on theme app extensions

packages/theme-check-node/src/config/load-config.spec.ts

+25-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import fs from 'node:fs/promises';
22
import path from 'node:path';
3-
import { describe, it, expect, afterEach, beforeEach, assert } from 'vitest';
3+
import { describe, it, expect, afterEach, beforeEach, assert, vi } from 'vitest';
44
import { loadConfig } from './load-config';
55
import {
66
allChecks,
@@ -19,6 +19,7 @@ import {
1919
} from '../test/test-helpers';
2020
import { thisNodeModuleRoot } from './installation-location';
2121
import { URI } from 'vscode-uri';
22+
import * as resolveModule from './resolve';
2223

2324
describe('Unit: loadConfig', () => {
2425
let tempDir: string;
@@ -34,8 +35,31 @@ describe('Unit: loadConfig', () => {
3435
it('loads the recommended config by default', async () => {
3536
const config = await loadConfig(undefined, __dirname);
3637
expect(config.checks).to.eql(recommended);
38+
expect(config.context).to.eql('theme');
3739
});
3840

41+
describe.each(['shopify.extension.toml', 'shopify.app.toml'])(
42+
'when the root contains a %s file',
43+
(fileName) => {
44+
beforeEach(async () => {
45+
const filePath = path.join(tempDir, fileName);
46+
await fs.writeFile(filePath, '', 'utf8');
47+
});
48+
49+
it('sets the context to app', async () => {
50+
const config = await loadConfig(undefined, tempDir);
51+
expect(config.context).to.eql('app');
52+
});
53+
54+
it('calls resolveConfig with theme-check:theme-app-extension', async () => {
55+
const spy = vi.spyOn(resolveModule, 'resolveConfig');
56+
await loadConfig(undefined, tempDir);
57+
expect(spy).toHaveBeenCalledWith('theme-check:theme-app-extension', true);
58+
vi.restoreAllMocks();
59+
});
60+
},
61+
);
62+
3963
it('extends the recommended config by default', async () => {
4064
const configPath = await createMockConfigFile(tempDir, `ignore: ['src/**']`);
4165
const config = await loadConfig(configPath, tempDir);

packages/theme-check-node/src/config/load-config.ts

+13-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { loadConfigDescription } from './load-config-description';
44
import { resolveConfig } from './resolve';
55
import { ModernIdentifier } from './types';
66
import { validateConfig } from './validation';
7+
import fs from 'fs/promises';
78

89
/**
910
* Given an absolute path to a config file, this function returns
@@ -22,7 +23,18 @@ export async function loadConfig(
2223
root: AbsolutePath,
2324
): Promise<Config> {
2425
if (!root) throw new Error('loadConfig cannot be called without a root argument');
25-
const configDescription = await resolveConfig(configPath ?? 'theme-check:recommended', true);
26+
let defaultChecks = 'theme-check:recommended';
27+
28+
if (!configPath) {
29+
const files = await fs.readdir(root);
30+
// *.extension.toml implies that we're already in the appropriate extensions
31+
// directory. *.app.toml implies that we're inside the root of an app.
32+
if (files.some((file) => file.endsWith('.extension.toml') || file.endsWith('.app.toml'))) {
33+
defaultChecks = 'theme-check:theme-app-extension';
34+
}
35+
}
36+
37+
const configDescription = await resolveConfig(configPath ?? defaultChecks, true);
2638
const config = await loadConfigDescription(configDescription, root);
2739
validateConfig(config);
2840
return config;

0 commit comments

Comments
 (0)