Merge pull request #5184 from Shopify/metafields-gitignore

Add `.shopify/metafields.json` to `.gitignore` by default

.changeset/mean-seas-hide.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+When the `.shopify/metafields.json` file gets created, the CLI now proposes to add it to `.gitignore` by default

packages/cli-kit/src/public/node/fs.test.ts

@@ -16,14 +16,17 @@ import {
   generateRandomNameForSubdirectory,
   readFileSync,
   glob,
+  detectEOL,
 } from './fs.js'
 import {joinPath} from './path.js'
 import {takeRandomFromArray} from '../common/array.js'
 import {describe, expect, test, vi} from 'vitest'
 import FastGlob from 'fast-glob'
+import * as os from 'os'
 
 vi.mock('../common/array.js')
 vi.mock('fast-glob')
+vi.mock('os')
 
 describe('inTemporaryDirectory', () => {
   test('ties the lifecycle of the temporary directory to the lifecycle of the callback', async () => {
@@ -283,3 +286,39 @@ describe('glob', () => {
     expect(FastGlob).toBeCalledWith('pattern', {dot: false})
   })
 })
+
+describe('detectEOL', () => {
+  test('detects the EOL of a file', async () => {
+    // Given
+    const fileContent = 'test\ncontent'
+
+    // When
+    const eol = detectEOL(fileContent)
+
+    // Then
+    expect(eol).toEqual('\n')
+  })
+
+  test('detects the EOL of a file with CRLF', async () => {
+    // Given
+    const fileContent = 'test\r\ncontent'
+
+    // When
+    const eol = detectEOL(fileContent)
+
+    // Then
+    expect(eol).toEqual('\r\n')
+  })
+
+  test('returns the default EOL if no EOL is found', async () => {
+    // Given
+    const fileContent = 'testcontent'
+    vi.mocked(os).EOL = '\n'
+
+    // When
+    const eol = detectEOL(fileContent)
+
+    // Then
+    expect(eol).toEqual('\n')
+  })
+})

packages/cli-kit/src/public/node/fs.ts

@@ -514,6 +514,43 @@ export async function glob(pattern: Pattern | Pattern[], options?: GlobOptions):
 export function pathToFileURL(path: string): URL {
   return pathToFile(path)
 }
+
+/**
+ * The operating system-specific end-of-line marker:
+ * - `\n` on POSIX
+ * - `\r\n` on Windows
+ */
+export type EOL = '\r\n' | '\n'
+
+/**
+ * Detects the end-of-line marker used in a string.
+ *
+ * @param content - file contents to analyze
+ *
+ * @returns The detected end-of-line marker
+ */
+export function detectEOL(content: string): EOL {
+  const match = content.match(/\r\n|\n/g)
+
+  if (!match) {
+    return defaultEOL()
+  }
+
+  const crlf = match.filter((eol) => eol === '\r\n').length
+  const lf = match.filter((eol) => eol === '\n').length
+
+  return crlf > lf ? '\r\n' : '\n'
+}
+
+/**
+ * Returns the operating system's end-of-line marker.
+ *
+ * @returns The OS-specific end-of-line marker
+ */
+export function defaultEOL(): EOL {
+  return os.EOL as EOL
+}
+
 /**
  * Find a file by walking parent directories.
  *

packages/theme/src/cli/services/metafields-pull.test.ts

@@ -6,7 +6,7 @@ import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/ses
 import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
 import {metafieldDefinitionsByOwnerType} from '@shopify/cli-kit/node/themes/api'
 import {describe, test, vi, beforeEach, expect, afterEach} from 'vitest'
-import {fileExists, inTemporaryDirectory, readFile} from '@shopify/cli-kit/node/fs'
+import {fileExists, inTemporaryDirectory, readFile, writeFileSync} from '@shopify/cli-kit/node/fs'
 
 vi.mock('../utilities/theme-store.js')
 vi.mock('../utilities/theme-ui.js')
@@ -83,6 +83,29 @@ describe('metafields-pull', () => {
     expect(capturedOutput.error()).toBeFalsy()
   })
 
+  test('should add metafields to .gitignore', async () => {
+    // Given
+    vi.mocked(metafieldDefinitionsByOwnerType).mockImplementation(() => {
+      return Promise.resolve([])
+    })
+
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      const gitIgnorePath = `${tmpDir}/.gitignore`
+      writeFileSync(gitIgnorePath, '.DS_Store\n.shopify/secrets.json')
+
+      // When
+      await metafieldsPull({path: tmpDir})
+
+      // Then
+      await expect(fileExists(gitIgnorePath)).resolves.toBe(true)
+      await expect(readFile(gitIgnorePath)).resolves.toBe(`.DS_Store\n.shopify/secrets.json\n.shopify`)
+    })
+
+    expect(capturedOutput.info()).toContain('Metafield definitions have been successfully downloaded.')
+    expect(capturedOutput.error()).toBeFalsy()
+  })
+
   test('should output to debug if some metafield definitions are not found', async () => {
     // Given
     vi.mocked(metafieldDefinitionsByOwnerType).mockImplementation((type: any, _session: AdminSession) => {

packages/theme/src/cli/services/metafields-pull.ts

@@ -6,7 +6,7 @@ import {AdminSession, ensureAuthenticatedThemes} from '@shopify/cli-kit/node/ses
 import {cwd, joinPath} from '@shopify/cli-kit/node/path'
 import {metafieldDefinitionsByOwnerType} from '@shopify/cli-kit/node/themes/api'
 import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui'
-import {mkdirSync, writeFileSync} from '@shopify/cli-kit/node/fs'
+import {detectEOL, fileExistsSync, mkdirSync, readFileSync, writeFileSync} from '@shopify/cli-kit/node/fs'
 import {outputDebug} from '@shopify/cli-kit/node/output'
 
 interface MetafieldsPullOptions {
@@ -161,5 +161,30 @@ function writeMetafieldDefinitionsToFile(path: string, content: unknown) {
   mkdirSync(shopifyDirectory)
 
   const filePath = joinPath(shopifyDirectory, 'metafields.json')
-  writeFileSync(filePath, JSON.stringify(content, null, 2))
+  const fileContent = JSON.stringify(content, null, 2)
+
+  if (!fileExistsSync(filePath)) {
+    addToGitIgnore(path, '.shopify')
+  }
+
+  writeFileSync(filePath, fileContent)
+}
+
+function addToGitIgnore(root: string, entry: string) {
+  const gitIgnorePath = joinPath(root, '.gitignore')
+
+  if (!fileExistsSync(gitIgnorePath)) {
+    // When the .gitignore file does not exist, the CLI should not be opinionated about creating it
+    return
+  }
+
+  const gitIgnoreContent = readFileSync(gitIgnorePath).toString()
+  const eol = detectEOL(gitIgnoreContent)
+
+  if (gitIgnoreContent.split(eol).includes(entry)) {
+    // The file already existing in the .gitignore
+    return
+  }
+
+  writeFileSync(gitIgnorePath, `${gitIgnoreContent}${eol}${entry}`)
 }