Merge pull request #4656 from Shopify/rm-pattern-warning

Remove the warning message from the ignore-module because some files can only be ignored using backward-compatible patterns
Shopify · Oct 17, 2024 · 27509eb · 27509eb
2 parents fa068c1 + c75c8e2
commit 27509eb
Show file tree

Hide file tree

Showing 5 changed files with 15 additions and 72 deletions.
diff --git a/.changeset/hungry-grapes-mate.md b/.changeset/hungry-grapes-mate.md
@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Remove the warning message from the ignore-module because some files can only be ignored using backward-compatible patterns
diff --git a/packages/theme/src/cli/utilities/asset-ignore.test.ts b/packages/theme/src/cli/utilities/asset-ignore.test.ts
@@ -1,6 +1,5 @@
 import {applyIgnoreFilters, getPatternsFromShopifyIgnore} from './asset-ignore.js'
 import {ReadOptions, fileExists, readFile} from '@shopify/cli-kit/node/fs'
-import {outputWarn} from '@shopify/cli-kit/node/output'
 import {test, describe, beforeEach, vi, expect} from 'vitest'
 
 vi.mock('@shopify/cli-kit/node/fs', async () => {
@@ -74,7 +73,7 @@ describe('asset-ignore', () => {
   describe('applyIgnoreFilters', () => {
     test(`returns entire list of checksums when there's no filter to apply`, async () => {
       // Given/When
-      const actualChecksums = await applyIgnoreFilters(checksums, {})
+      const actualChecksums = applyIgnoreFilters(checksums, {})
 
       // Then
       expect(actualChecksums).toEqual(checksums)
@@ -94,7 +93,7 @@ describe('asset-ignore', () => {
       }
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([{key: 'assets/basic.css', checksum: '00000000000000000000000000000000'}])
@@ -105,7 +104,7 @@ describe('asset-ignore', () => {
       const options = {ignore: ['config/*', 'templates/*', 'assets/image.png']}
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([
@@ -120,7 +119,7 @@ describe('asset-ignore', () => {
       const options = {only: ['config/*', 'assets/image.png']}
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([
@@ -135,7 +134,7 @@ describe('asset-ignore', () => {
       const options = {ignore: ['*.css']}
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([
@@ -153,7 +152,7 @@ describe('asset-ignore', () => {
       const options = {only: ['templates/*.json']}
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([
@@ -167,43 +166,12 @@ describe('asset-ignore', () => {
       const options = {only: ['templates/**/*.json']}
 
       // When
-      const actualChecksums = await applyIgnoreFilters(checksums, options)
+      const actualChecksums = applyIgnoreFilters(checksums, options)
 
       // Then
       expect(actualChecksums).toEqual([
         {key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'},
       ])
     })
-
-    test(`only outputs glob pattern subdirectory warnings for the templates folder`, async () => {
-      // Given
-      const options = {only: ['assets/*.json', 'config/*.json', 'sections/*.json']}
-      // When
-      await applyIgnoreFilters(checksums, options)
-      // Then
-      expect(vi.mocked(outputWarn)).not.toHaveBeenCalled()
-    })
-
-    test(`outputs warnings when there are glob pattern modifications required for subdirectories`, async () => {
-      // Given
-      const options = {only: ['templates/*.json']}
-
-      // When
-      await applyIgnoreFilters(checksums, options)
-
-      // Then
-      expect(vi.mocked(outputWarn)).toHaveBeenCalledTimes(1)
-    })
-
-    test('only outputs a single warning for duplicated glob patterns', async () => {
-      // Given
-      const options = {only: ['templates/*.json'], ignore: ['templates/*.json']}
-
-      // When
-      await applyIgnoreFilters(checksums, options)
-
-      // Then
-      expect(vi.mocked(outputWarn)).toHaveBeenCalledTimes(1)
-    })
   })
 })
diff --git a/packages/theme/src/cli/utilities/asset-ignore.ts b/packages/theme/src/cli/utilities/asset-ignore.ts
@@ -1,5 +1,5 @@
 import {fileExists, readFile, matchGlob as originalMatchGlob} from '@shopify/cli-kit/node/fs'
-import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output'
+import {outputDebug} from '@shopify/cli-kit/node/output'
 import {joinPath} from '@shopify/cli-kit/node/path'
 
 const SHOPIFY_IGNORE = '.shopifyignore'
@@ -13,8 +13,6 @@ export function applyIgnoreFilters<T extends {key: string}>(
   const ignoreOptions = options.ignore ?? []
   const onlyOptions = options.only ?? []
 
-  raiseWarningForNonExplicitGlobPatterns([...shopifyIgnore, ...ignoreOptions, ...onlyOptions])
-
   return files
     .filter(filterBy(shopifyIgnore, '.shopifyignore'))
     .filter(filterBy(ignoreOptions, '--ignore'))
@@ -73,20 +71,6 @@ function matchGlob(key: string, pattern: string) {
   return false
 }
 
-export function raiseWarningForNonExplicitGlobPatterns(patterns: string[]) {
-  const allPatterns = new Set(patterns)
-  allPatterns.forEach((pattern) => {
-    if (shouldReplaceGlobPattern(pattern)) {
-      outputWarn(
-        `Warning: The pattern '${pattern}' does not include subdirectories. To maintain backwards compatibility, we have modified your pattern to ${pattern.replace(
-          templatesRegex,
-          'templates/**/*$1',
-        )} to explicitly include subdirectories.`,
-      )
-    }
-  })
-}
-
 function shouldReplaceGlobPattern(pattern: string): boolean {
   return templatesRegex.test(pattern)
 }

diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts
@@ -7,11 +7,7 @@ import {
   partitionThemeFiles,
   readThemeFile,
 } from './theme-fs.js'
-import {
-  getPatternsFromShopifyIgnore,
-  applyIgnoreFilters,
-  raiseWarningForNonExplicitGlobPatterns,
-} from './asset-ignore.js'
+import {getPatternsFromShopifyIgnore, applyIgnoreFilters} from './asset-ignore.js'
 import {removeFile, writeFile} from '@shopify/cli-kit/node/fs'
 import {test, describe, expect, vi, beforeEach} from 'vitest'
 import chokidar from 'chokidar'
@@ -279,7 +275,6 @@ describe('theme-fs', () => {
       const themeFileSystem = mountThemeFileSystem(root, options)
       await themeFileSystem.ready()
 
-      expect(raiseWarningForNonExplicitGlobPatterns).toHaveBeenCalledOnce()
       expect(getPatternsFromShopifyIgnore).toHaveBeenCalledWith(root)
       expect(themeFileSystem.applyIgnoreFilters(files)).toEqual([{key: 'assets/file.json'}])
       expect(applyIgnoreFilters).toHaveBeenCalledWith(files, {

diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts
@@ -1,9 +1,5 @@
 import {calculateChecksum} from './asset-checksum.js'
-import {
-  applyIgnoreFilters,
-  raiseWarningForNonExplicitGlobPatterns,
-  getPatternsFromShopifyIgnore,
-} from './asset-ignore.js'
+import {applyIgnoreFilters, getPatternsFromShopifyIgnore} from './asset-ignore.js'
 import {Notifier} from './notifier.js'
 import {createSyncingCatchError} from './errors.js'
 import {DEFAULT_IGNORE_PATTERNS, timestampDateFormat} from '../constants.js'
@@ -86,11 +82,6 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
     .then((filesPaths) => Promise.all([getPatternsFromShopifyIgnore(root), ...filesPaths.map(read)]))
     .then(([ignoredPatterns]) => {
       filterPatterns.ignoreFromFile.push(...ignoredPatterns)
-      raiseWarningForNonExplicitGlobPatterns([
-        ...filterPatterns.ignoreFromFile,
-        ...filterPatterns.ignore,
-        ...filterPatterns.only,
-      ])
     })
 
   const getKey = (filePath: string) => relativePath(root, filePath)