Skip to content

Commit

Permalink
Merge pull request #4642 from montalvomiguelo/fix-ignore-patterns
Browse files Browse the repository at this point in the history
Fix ignore patterns
  • Loading branch information
karreiro authored Oct 16, 2024
2 parents fac882b + cd35797 commit 5a7ef7a
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 14 deletions.
5 changes: 5 additions & 0 deletions .changeset/wicked-apricots-shake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/theme': patch
---

Fix ignore patterns
2 changes: 1 addition & 1 deletion packages/theme/src/cli/utilities/asset-ignore.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ describe('asset-ignore', () => {
'sections/*',
'templates/*',
'config/*_data.json',
'.*settings_schema.json',
'/settings_schema/',
],
}

Expand Down
32 changes: 20 additions & 12 deletions packages/theme/src/cli/utilities/asset-ignore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {outputDebug, outputWarn} from '@shopify/cli-kit/node/output'
import {joinPath} from '@shopify/cli-kit/node/path'

const SHOPIFY_IGNORE = '.shopifyignore'
const templatesRegex = /templates\/\*(\.(json|liquid))?$/

export function applyIgnoreFilters<T extends {key: string}>(
files: T[],
Expand All @@ -24,7 +25,9 @@ function filterBy(patterns: string[], type: string, invertMatch = false) {
return ({key}: {key: string}) => {
if (patterns.length === 0) return true

const match = patterns.some((pattern) => matchGlob(key, pattern) || regexMatch(key, pattern))
const match = patterns.some(
(pattern) => matchGlob(key, pattern) || (isRegex(pattern) && regexMatch(key, asRegex(pattern))),
)
const shouldIgnore = invertMatch ? !match : match

if (shouldIgnore) {
Expand Down Expand Up @@ -64,7 +67,7 @@ function matchGlob(key: string, pattern: string) {
// replace '/*.' with '/**/*.' to emulate Shopify CLI 2.x behavior, as it was
// based on 'File.fnmatch'.
if (shouldReplaceGlobPattern(pattern)) {
return originalMatchGlob(key, pattern.replace('/*.', '/**/*.'), matchOpts)
return originalMatchGlob(key, pattern.replace(templatesRegex, 'templates/**/*$1'), matchOpts)
}

return false
Expand All @@ -76,23 +79,28 @@ export function raiseWarningForNonExplicitGlobPatterns(patterns: string[]) {
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 pattern.includes('/*.') && !pattern.includes('/**/*.') && pattern.includes('templates/')
return templatesRegex.test(pattern)
}

function regexMatch(key: string, pattern: string) {
try {
return key.match(pattern)
// eslint-disable-next-line no-catch-all/no-catch-all
} catch {
return false
}
function regexMatch(key: string, regex: RegExp) {
return regex.test(key)
}

// https://github.com/Shopify/cli/blob/2ddbd3eee70c50814c5527d6d3eeb7ca601de5f8/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/filter/path_matcher.rb#L17
function isRegex(pattern: string) {
return pattern.startsWith('/') && pattern.endsWith('/')
}

// https://github.com/Shopify/cli/blob/2ddbd3eee70c50814c5527d6d3eeb7ca601de5f8/packages/cli-kit/assets/cli-ruby/lib/shopify_cli/theme/filter/path_matcher.rb#L21
function asRegex(pattern: string) {
return new RegExp(pattern.slice(1, -1))
}
2 changes: 1 addition & 1 deletion packages/theme/src/cli/utilities/theme-downloader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ describe('theme-downloader', () => {

test('downloads missing remote files', async () => {
// Given
const downloadOptions = {nodelete: false, only: ['release'], ignore: ['release/ignoreme']}
const downloadOptions = {nodelete: false, only: ['/release/'], ignore: ['release/ignoreme']}
const fileToDownload = {key: 'release/downloadme', checksum: '1'}
const files = new Map<string, ThemeAsset>([
['release/alreadyexists', {checksum: '2', value: 'content', key: 'release/alreadyexists'}],
Expand Down

0 comments on commit 5a7ef7a

Please sign in to comment.