Skip to content

Commit

Permalink
Improve Glob Pattern subdirectory mismatch error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesmengo committed Apr 5, 2024
1 parent 1bded45 commit 72a46ba
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/spotty-badgers-attack.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

Improve Glob Pattern subdirectory mismatch error handling
31 changes: 30 additions & 1 deletion packages/theme/src/cli/utilities/asset-ignore.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {applyIgnoreFilters} from './asset-ignore.js'
import {ReadOptions, fileExists, readFile} from '@shopify/cli-kit/node/fs'
import {ReadOptions, fileExists, matchGlob, readFile} from '@shopify/cli-kit/node/fs'
import {outputWarn} from '@shopify/cli-kit/node/output'
import {joinPath} from '@shopify/cli-kit/node/path'
import {test, describe, beforeEach, vi, expect} from 'vitest'

Expand All @@ -14,6 +15,7 @@ vi.mock('@shopify/cli-kit/node/fs', async () => {
})

vi.mock('@shopify/cli-kit/node/path')
vi.mock('@shopify/cli-kit/node/output')

describe('applyIgnoreFilters', () => {
const checksums = [
Expand Down Expand Up @@ -129,4 +131,31 @@ describe('applyIgnoreFilters', () => {
{key: 'templates/customers/account.json', checksum: '7777777777777777777777777777777'},
])
})

test(`does not output warnings when there are no glob pattern modifications required for subdirectories`, async () => {
// Given
const options = {only: ['templates/*.json']}

const filteredChecksums = checksums.filter(({key}) => !matchGlob(key, 'templates/**/*.json'))

// When
await applyIgnoreFilters(filteredChecksums, themeFileSystem, 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']}
const numFilesInSubDirectory = checksums.filter(
({key}) => matchGlob(key, 'templates/**/*.json') && !matchGlob(key, 'templates/*.json'),
).length

// When
await applyIgnoreFilters(checksums, themeFileSystem, options)

// Then
expect(vi.mocked(outputWarn)).toHaveBeenCalledTimes(numFilesInSubDirectory)
})
})
14 changes: 11 additions & 3 deletions packages/theme/src/cli/utilities/asset-ignore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,17 @@ function matchGlob(key: string, pattern: string) {
// When the the standard match fails and the pattern includes '/*.', we
// replace '/*.' with '/**/*.' to emulate Shopify CLI 2.x behavior, as it was
// based on 'File.fnmatch'.
if (pattern.includes('/*.')) {
outputWarn(`The pattern ${pattern} is unsafe, please use a valid regex or glob.`)
return originalMatchGlob(key, pattern.replace('/*.', '/**/*.'))
if (pattern.includes('/*.') && !pattern.includes('/**/*.')) {
const newPatternMatch = originalMatchGlob(key, pattern.replace('/*.', '/**/*.'))
if (newPatternMatch) {
outputWarn(
`Warning: File ${key} does not match the pattern '${pattern}'. To maintain backwards compatibility, we have modified your pattern to ${pattern.replace(
'/*.',
'/**/*.',
)} to explicitly include subdirectories.`,
)
}
return newPatternMatch
}

return false
Expand Down

0 comments on commit 72a46ba

Please sign in to comment.