From 2720923007c858506e9c021320fb0b8dac67cb9f Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Wed, 23 Oct 2024 15:40:14 -0400 Subject: [PATCH] Fix bug related to using negated patterns in shopify ignore This commit fixes an issue where negated patterns in the shopify ignore file were not being respected. --- .changeset/fresh-crabs-kick.md | 5 + .../src/cli/utilities/asset-ignore.test.ts | 110 ++++++++++++++++++ .../theme/src/cli/utilities/asset-ignore.ts | 34 +++++- .../theme/src/cli/utilities/theme-fs.test.ts | 1 + .../theme-fs/theme-fs-mock-factory.ts | 1 + 5 files changed, 147 insertions(+), 4 deletions(-) create mode 100644 .changeset/fresh-crabs-kick.md diff --git a/.changeset/fresh-crabs-kick.md b/.changeset/fresh-crabs-kick.md new file mode 100644 index 0000000000..bfd09df870 --- /dev/null +++ b/.changeset/fresh-crabs-kick.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli': patch +--- + +Fix files ignored when using negate patterns diff --git a/packages/theme/src/cli/utilities/asset-ignore.test.ts b/packages/theme/src/cli/utilities/asset-ignore.test.ts index d4668d5932..c4ba7ea1ea 100644 --- a/packages/theme/src/cli/utilities/asset-ignore.test.ts +++ b/packages/theme/src/cli/utilities/asset-ignore.test.ts @@ -174,4 +174,114 @@ describe('asset-ignore', () => { ]) }) }) + + describe('applyIgnoreFilters with negated patterns', () => { + test(`only negating a file does not produce side effects of other ignored files`, () => { + const options = { + ignoreFromFile: ['!assets/basic.css'], + } + + const actualChecksums = applyIgnoreFilters(checksums, options) + + expect(actualChecksums).toEqual([ + {key: 'assets/basic.css', checksum: '00000000000000000000000000000000'}, + {key: 'assets/complex.css', checksum: '11111111111111111111111111111111'}, + {key: 'assets/image.png', checksum: '22222222222222222222222222222222'}, + {key: 'config/settings_data.json', checksum: '33333333333333333333333333333333'}, + {key: 'config/settings_schema.json', checksum: '44444444444444444444444444444444'}, + {key: 'sections/announcement-bar.liquid', checksum: '55555555555555555555555555555555'}, + {key: 'templates/404.json', checksum: '6666666666666666666666666666666'}, + {key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'}, + ]) + }) + test(`negating a specific file overrides ignoring one`, () => { + // Given + const options = { + ignoreFromFile: ['assets/*.css', '!assets/basic.css'], + } + + // When + const actualChecksums = applyIgnoreFilters(checksums, options) + + // Then + expect(actualChecksums).toEqual([ + {key: 'assets/image.png', checksum: '22222222222222222222222222222222'}, + {key: 'config/settings_data.json', checksum: '33333333333333333333333333333333'}, + {key: 'config/settings_schema.json', checksum: '44444444444444444444444444444444'}, + {key: 'sections/announcement-bar.liquid', checksum: '55555555555555555555555555555555'}, + {key: 'templates/404.json', checksum: '6666666666666666666666666666666'}, + {key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'}, + {key: 'assets/basic.css', checksum: '00000000000000000000000000000000'}, + ]) + }) + + test('should not ignore files matching a negated pattern', () => { + const ignorePatterns = [ + 'assets/basic.css', + 'sections/*.json', + 'templates/*.json', + 'templates/**/*.json', + 'config/*.json', + '!config/*_schema.json', + ] + + const actualChecksums = applyIgnoreFilters(checksums, {ignoreFromFile: ignorePatterns}) + + expect(actualChecksums).toEqual([ + {key: 'assets/complex.css', checksum: '11111111111111111111111111111111'}, + {key: 'assets/image.png', checksum: '22222222222222222222222222222222'}, + {key: 'sections/announcement-bar.liquid', checksum: '55555555555555555555555555555555'}, + {key: 'config/settings_schema.json', checksum: '44444444444444444444444444444444'}, + ]) + }) + }) + describe('applyIgnoreFilters with negated ignore and only options', () => { + test(`negating a specific file in ignore option overrides ignoring one`, () => { + const options = { + ignore: ['assets/*.css', '!assets/basic.css'], + } + + const actualChecksums = applyIgnoreFilters(checksums, options) + + expect(actualChecksums).toEqual([ + {key: 'assets/image.png', checksum: '22222222222222222222222222222222'}, + {key: 'config/settings_data.json', checksum: '33333333333333333333333333333333'}, + {key: 'config/settings_schema.json', checksum: '44444444444444444444444444444444'}, + {key: 'sections/announcement-bar.liquid', checksum: '55555555555555555555555555555555'}, + {key: 'templates/404.json', checksum: '6666666666666666666666666666666'}, + {key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'}, + {key: 'assets/basic.css', checksum: '00000000000000000000000000000000'}, + ]) + }) + + test(`negating a specific file in only option properly overrides and ignores it`, () => { + const options = { + only: ['assets/*.css', '!assets/basic.css'], + } + + const actualChecksums = applyIgnoreFilters(checksums, options) + + expect(actualChecksums).toEqual([{key: 'assets/complex.css', checksum: '11111111111111111111111111111111'}]) + }) + }) + describe('applyIgnoreFilters do not return duplicates', () => { + test(`should not return duplicates when negated patterns are used`, () => { + const options = { + ignoreFromFile: ['assets/*.css', '!assets/basic.css'], + ignore: ['!assets/basic.css'], + } + + const actualChecksums = applyIgnoreFilters(checksums, options) + + expect(actualChecksums).toEqual([ + {key: 'assets/image.png', checksum: '22222222222222222222222222222222'}, + {key: 'config/settings_data.json', checksum: '33333333333333333333333333333333'}, + {key: 'config/settings_schema.json', checksum: '44444444444444444444444444444444'}, + {key: 'sections/announcement-bar.liquid', checksum: '55555555555555555555555555555555'}, + {key: 'templates/404.json', checksum: '6666666666666666666666666666666'}, + {key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'}, + {key: 'assets/basic.css', checksum: '00000000000000000000000000000000'}, + ]) + }) + }) }) diff --git a/packages/theme/src/cli/utilities/asset-ignore.ts b/packages/theme/src/cli/utilities/asset-ignore.ts index 337896db4c..8bba79a5a6 100644 --- a/packages/theme/src/cli/utilities/asset-ignore.ts +++ b/packages/theme/src/cli/utilities/asset-ignore.ts @@ -1,3 +1,4 @@ +import {uniqBy} from '@shopify/cli-kit/common/array' import {fileExists, readFile, matchGlob as originalMatchGlob} from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' import {joinPath} from '@shopify/cli-kit/node/path' @@ -13,10 +14,26 @@ export function applyIgnoreFilters( const ignoreOptions = options.ignore ?? [] const onlyOptions = options.only ?? [] - return files - .filter(filterBy(shopifyIgnore, '.shopifyignore')) - .filter(filterBy(ignoreOptions, '--ignore')) - .filter(filterBy(onlyOptions, '--only', true)) + const [normalShopifyPatterns = [], negatedShopifyPatterns = []] = filterRegexValues(shopifyIgnore) + const [normalIgnorePatterns = [], negatedIgnorePatterns = []] = filterRegexValues(ignoreOptions) + const [normalOnlyPatterns = [], negatedOnlyPatterns = []] = filterRegexValues(onlyOptions) + + let filteredFiles = files.filter(filterBy(normalShopifyPatterns, '.shopifyignore')) + filteredFiles = filteredFiles.filter(filterBy(normalIgnorePatterns, '--ignore')) + filteredFiles = filteredFiles.filter(filterBy(normalOnlyPatterns, '--only', true)) + + if (negatedShopifyPatterns.length > 0) { + filteredFiles = filteredFiles.concat(files.filter(filterBy(negatedShopifyPatterns, '.shopifyignore', true))) + } + if (negatedIgnorePatterns.length > 0) { + filteredFiles = filteredFiles.concat(files.filter(filterBy(negatedIgnorePatterns, '--ignore', true))) + } + if (negatedOnlyPatterns.length > 0) { + filteredFiles = filteredFiles.filter(filterBy(negatedOnlyPatterns, '--only')) + } + + const uniqueFiles = uniqBy(filteredFiles, (file) => file.key) + return uniqueFiles } function filterBy(patterns: string[], type: string, invertMatch = false) { @@ -51,6 +68,15 @@ export async function getPatternsFromShopifyIgnore(root: string) { .filter((line) => line && !line.startsWith('#')) } +function filterRegexValues(regexList: string[]) { + const negatedPatterns = regexList + .filter((regexList) => regexList.startsWith('!')) + .map((regexList) => regexList.slice(1)) + const normalPatterns = regexList.filter((regexList) => !regexList.startsWith('!')) + + return [normalPatterns, negatedPatterns] +} + function matchGlob(key: string, pattern: string) { const matchOpts = { matchBase: true, diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts index fa81b0688a..a2c1298568 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -293,6 +293,7 @@ describe('theme-fs', () => { // When const content = await readThemeFile(root, key) + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing const contentJson = JSON.parse(content?.toString() || '') // Then diff --git a/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts b/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts index 83f531b383..ce559833cb 100644 --- a/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts +++ b/packages/theme/src/cli/utilities/theme-fs/theme-fs-mock-factory.ts @@ -18,6 +18,7 @@ export function fakeThemeFileSystem( files.set(asset.key, asset) }, read: async (fileKey: string) => { + // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing return files.get(fileKey)?.value || files.get(fileKey)?.attachment }, addEventListener: () => {},