From c75c8e201b2894a847c997eaa214c7ae81411c0b Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 16 Oct 2024 11:23:37 +0200 Subject: [PATCH] Remove the warning message from the ignore-module because some files can only be ignored using backward-compatible patterns --- .changeset/hungry-grapes-mate.md | 5 ++ .../src/cli/utilities/asset-ignore.test.ts | 46 +++---------------- .../theme/src/cli/utilities/asset-ignore.ts | 18 +------- .../theme/src/cli/utilities/theme-fs.test.ts | 7 +-- packages/theme/src/cli/utilities/theme-fs.ts | 11 +---- 5 files changed, 15 insertions(+), 72 deletions(-) create mode 100644 .changeset/hungry-grapes-mate.md diff --git a/.changeset/hungry-grapes-mate.md b/.changeset/hungry-grapes-mate.md new file mode 100644 index 0000000000..959d6d7a66 --- /dev/null +++ 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 index 04ac0f3910..d4668d5932 100644 --- 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 index 37e42c5dff..337896db4c 100644 --- 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( 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 index 94787483f4..6cdd1b5924 100644 --- 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 index f08cb26f0d..24a27041ac 100644 --- 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)