Skip to content

Commit ea80152

Browse files
authored
Merge pull request #861 from Shopify/gg-fix-app-ext-theme-check
Fix bug where theme check would fail for theme app extensions
2 parents ea9f245 + 449d162 commit ea80152

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)