Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve mechanism that adds .shopify to .gitignore to avoid duplicate entries #5314

Merged
merged 1 commit into from
Jan 31, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/curvy-kangaroos-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@shopify/cli-kit': patch
'@shopify/theme': patch
---

Improve mechanism that adds `.shopify` to `.gitignore` to avoid duplicate entries
1 change: 1 addition & 0 deletions packages/cli-kit/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@
"gradient-string": "2.0.2",
"graphql": "16.8.1",
"graphql-request": "5.2.0",
"ignore": "6.0.2",
"ink": "4.4.1",
"is-interactive": "2.0.0",
"jose": "5.9.6",
Expand Down
34 changes: 34 additions & 0 deletions packages/cli-kit/src/public/node/git.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -444,4 +444,38 @@ describe('addToGitIgnore()', () => {
expect(gitIgnoreContent).toBe('node_modules\ndist\n.shopify\n')
})
})

test('does nothing when .shopify/* pattern exists in .gitignore', async () => {
await inTemporaryDirectory(async (tmpDir) => {
// Given
const gitIgnorePath = `${tmpDir}/.gitignore`
const gitIgnoreContent = '.shopify/*\nnode_modules\n'

writeFileSync(gitIgnorePath, gitIgnoreContent)

// When
git.addToGitIgnore(tmpDir, '.shopify')

// Then
const actualContent = readFileSync(gitIgnorePath).toString()
expect(actualContent).toBe(gitIgnoreContent)
})
})

test('does nothing when .shopify/** pattern exists in .gitignore', async () => {
await inTemporaryDirectory(async (tmpDir) => {
// Given
const gitIgnorePath = `${tmpDir}/.gitignore`
const gitIgnoreContent = '.shopify/**\nnode_modules\n'

writeFileSync(gitIgnorePath, gitIgnoreContent)

// When
git.addToGitIgnore(tmpDir, '.shopify')

// Then
const actualContent = readFileSync(gitIgnorePath).toString()
expect(actualContent).toBe(gitIgnoreContent)
})
})
})
11 changes: 9 additions & 2 deletions packages/cli-kit/src/public/node/git.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {cwd, joinPath} from './path.js'
import {runWithTimer} from './metadata.js'
import {outputContent, outputToken, outputDebug} from '../../public/node/output.js'
import git, {TaskOptions, SimpleGitProgressEvent, DefaultLogFields, ListLogLine, SimpleGit} from 'simple-git'
import ignore from 'ignore'

/**
* Initialize a git repository at the given directory.
Expand Down Expand Up @@ -83,8 +84,14 @@ export function addToGitIgnore(root: string, entry: string): void {
const gitIgnoreContent = readFileSync(gitIgnorePath).toString()
const eol = detectEOL(gitIgnoreContent)

if (gitIgnoreContent.split(eol).some((line) => line.trim() === entry.trim())) {
// The file already existing in the .gitignore
const lines = gitIgnoreContent.split(eol).map((line) => line.trim())
const ignoreManager = ignore.default({allowRelativePaths: true}).add(lines)

const isIgnoredEntry = ignoreManager.ignores(joinPath(entry))
const isIgnoredEntryAsDir = ignoreManager.ignores(joinPath(entry, 'ignored.txt'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's ignored.txt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a mocked file entry that supports the following two test scenarios:

- does nothing when .shopify/* pattern exists in .gitignore
- does nothing when .shopify/** pattern exists in .gitignore

The ignore package doesn't interact with the file system; it just analyzes paths and patterns. This mocked entry helps us ensure that .shopify/* and .shopify/** are really working as expected.

const isAlreadyIgnored = isIgnoredEntry || isIgnoredEntryAsDir
if (isAlreadyIgnored) {
// The file is already ignored by an existing pattern
return
}

Expand Down
7 changes: 5 additions & 2 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.