Skip to content

Commit f592935

Browse files
committed
Check for duplicate files and throw an error
1 parent b1870c1 commit f592935

12 files changed

Lines changed: 90 additions & 2 deletions

File tree

src/commands/theme/component/copy.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import BaseCommand from '../../../utilities/base-command.js'
1313
import { copyFileIfChanged } from '../../../utilities/files.js';
1414
import Flags from '../../../utilities/flags.js'
1515
import { getManifest } from '../../../utilities/manifest.js'
16-
import { getCollectionNodes } from '../../../utilities/nodes.js'
16+
import { getCollectionNodes, getDuplicateFiles } from '../../../utilities/nodes.js'
1717
import { getNameFromPackageJson, getVersionFromPackageJson } from '../../../utilities/package-json.js'
1818
import { isComponentRepo, isThemeRepo } from '../../../utilities/validate.js'
1919

@@ -54,6 +54,21 @@ export default class Copy extends BaseCommand {
5454

5555
const manifest = getManifest(path.join(destinationDir, 'component.manifest.json'))
5656
const sourceNodes = await getCollectionNodes(currentDir)
57+
const manifest = getManifest(path.join(themeDir, 'component.manifest.json'))
58+
const componentNodes = await getCollectionNodes(currentDir)
59+
60+
const duplicates = getDuplicateFiles(componentNodes);
61+
62+
if (duplicates.size > 0) {
63+
const message: string[] = []
64+
duplicates.forEach((nodes, key) => {
65+
message.push(`Warning: Found duplicate files for ${key}:`)
66+
nodes.forEach(node => {
67+
message.push(` - ${node.file}`)
68+
})
69+
});
70+
this.error(message.join('\n'))
71+
}
5772

5873
if (manifest.collections[sourceName].version !== sourceVersion) {
5974
this.error(`Version mismatch: Expected ${sourceVersion} but found ${manifest.collections[sourceName].version}. Run "shopify theme component map" to update the component.manifest.json file.`);

src/commands/theme/component/manifest.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import BaseCommand from '../../../utilities/base-command.js'
1313
import Flags from '../../../utilities/flags.js'
1414
import { getLastCommitHash } from '../../../utilities/git.js'
1515
import { ManifestOptions, generateManifestFile, getManifest } from '../../../utilities/manifest.js'
16-
import { getCollectionNodes, getThemeNodes } from '../../../utilities/nodes.js'
16+
import { getCollectionNodes, getDuplicateFiles, getThemeNodes } from '../../../utilities/nodes.js'
1717
import { sortObjectKeys } from '../../../utilities/objects.js'
1818
import { getNameFromPackageJson, getVersionFromPackageJson } from '../../../utilities/package-json.js'
1919
import { LiquidNode } from '../../../utilities/types.js'
@@ -69,6 +69,19 @@ export default class Manifest extends BaseCommand {
6969

7070
const sourceNodes = await getCollectionNodes(sourceDir)
7171

72+
const duplicates = getDuplicateFiles(sourceNodes);
73+
74+
if (duplicates.size > 0) {
75+
const message: string[] = []
76+
duplicates.forEach((nodes, key) => {
77+
message.push(`Warning: Found duplicate files for ${key}:`)
78+
nodes.forEach(node => {
79+
message.push(` - ${node.file}`)
80+
})
81+
});
82+
this.error(message.join('\n'))
83+
}
84+
7285
let destinationNodes: LiquidNode[]
7386
let destinationName: string
7487

src/utilities/nodes.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,26 @@ export async function getCollectionNodes(collectionDir: string): Promise<LiquidN
116116
return Promise.all([...collectionSnippets, ...collectionComponents, ...collectionAssets, ...collectionScripts, ...collectionSetup])
117117
}
118118

119+
export function getDuplicateFiles(nodes: LiquidNode[]): Map<string, LiquidNode[]> {
120+
const duplicateMap = new Map<string, LiquidNode[]>();
121+
122+
nodes.forEach(node => {
123+
if (node.themeFolder !== 'snippets' && node.themeFolder !== 'assets') return;
124+
const key = `${node.themeFolder}/${node.name}`;
125+
if (!duplicateMap.has(key)) {
126+
duplicateMap.set(key, [node]);
127+
} else {
128+
duplicateMap.get(key)!.push(node);
129+
}
130+
});
131+
132+
// Filter to only return entries with duplicates
133+
return new Map(
134+
Array.from(duplicateMap.entries())
135+
.filter(([_, nodes]) => nodes.length > 1)
136+
);
137+
}
138+
119139
export async function getThemeNodes(themeDir: string): Promise<LiquidNode[]> {
120140
const entryNodes = globSync(path.join(themeDir, '{layout,sections,blocks,templates}', '*.liquid'), { absolute: true })
121141
.map(file => {

test/commands/theme/component/manifest.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -357,4 +357,15 @@ describe('theme component manifest', () => {
357357
const data = JSON.parse(fs.readFileSync(path.join(testThemePath, 'component.manifest.json'), 'utf8'))
358358
expect(data.collections['@archetype-themes/test-collection'].commit).to.equal(expectedHash)
359359
})
360+
361+
it('should throw an error if there are duplicate files in the source collection', async () => {
362+
const src = path.join(testCollectionPath, 'components', 'duplicate', 'duplicate.liquid')
363+
const dest = path.join(testCollectionPath, 'components', 'duplicate', 'snippets', 'duplicate.liquid')
364+
365+
fs.cpSync(src, dest, {recursive: true})
366+
367+
const {error} = await runCommand(['theme', 'component', 'manifest', testThemePath])
368+
expect(error).to.be.instanceOf(Error)
369+
expect(error?.message).to.include('Error: Namespace conflict in source component collection with')
370+
})
360371
})

test/fixtures/collection/components/duplicate/duplicate.liquid

Whitespace-only changes.
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
{
2+
"collections": {
3+
"@archetype-themes/test-collection": {
4+
"version": "1.0.0"
5+
}
6+
},
7+
"files": {
8+
"snippets": {
9+
"belongs-only-to-this-collection.liquid": "@collection",
10+
"uses-component-from-other-collection.liquid": "@collection",
11+
"used-by-other-collection-already-installed.liquid": "@archetype-themes/test-collection",
12+
"used-by-other-collection-to-be-copied.liquid": "@archetype-themes/test-collection",
13+
"used-by-other-collection-to-be-copied.snippet.liquid": "@archetype-themes/test-collection"
14+
},
15+
"assets": {
16+
"belongs-only-to-this-collection.css": "@collection",
17+
"used-by-other-collection-already-installed.css": "@archetype-themes/test-collection",
18+
"used-by-other-collection-to-be-copied.css": "@archetype-themes/test-collection",
19+
"shared-script-used-by-other-collection.js": "@archetype-themes/test-collection"
20+
}
21+
}
22+
}

test/fixtures/test-collection-b/components/belongs-only-to-this-collection/assets/belongs-only-to-this-collection.css

Whitespace-only changes.

test/fixtures/test-collection-b/components/belongs-only-to-this-collection/belongs-only-to-this-collection.liquid

Whitespace-only changes.

test/fixtures/test-collection-b/components/used-by-other-collection-already-installed/used-by-other-collection-already-installed.liquid

Whitespace-only changes.
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
{% render 'used-by-other-collection-already-installed' %}
2+
{% render 'used-by-other-collection-not-installed' %}

0 commit comments

Comments
 (0)