From 404fa77c901afaa552bdc14da211192b2c4585d3 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Mon, 13 Jan 2025 11:24:26 +0100 Subject: [PATCH 1/4] When the `.shopify/metafields.json` file gets created, the CLI now proposes to add it to `.gitignore` by default --- .changeset/mean-seas-hide.md | 5 ++++ .../src/cli/services/metafields-pull.test.ts | 26 ++++++++++++++++- .../theme/src/cli/services/metafields-pull.ts | 28 +++++++++++++++++-- 3 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 .changeset/mean-seas-hide.md diff --git a/.changeset/mean-seas-hide.md b/.changeset/mean-seas-hide.md new file mode 100644 index 00000000000..b350cadda0e --- /dev/null +++ b/.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 diff --git a/packages/theme/src/cli/services/metafields-pull.test.ts b/packages/theme/src/cli/services/metafields-pull.test.ts index ea96eda111f..88a93c1b7ef 100644 --- a/packages/theme/src/cli/services/metafields-pull.test.ts +++ b/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,30 @@ describe('metafields-pull', () => { expect(capturedOutput.error()).toBeFalsy() }) + test('should add metafields to .gitignore', async () => { + // Given + vi.mocked(metafieldDefinitionsByOwnerType).mockImplementation((type: any, _session: AdminSession) => { + if (type !== 'PRODUCT') return Promise.resolve([]) + return Promise.resolve([fakeMetafieldDefinition]) + }) + + await inTemporaryDirectory(async (tmpDir) => { + // Given + const gitIgnorePath = `${tmpDir}/.gitignore` + writeFileSync(gitIgnorePath, '.DS_Store') + + // When + await metafieldsPull({path: tmpDir}) + + // Then + await expect(fileExists(gitIgnorePath)).resolves.toBe(true) + await expect(readFile(gitIgnorePath)).resolves.toBe('.DS_Store\n.shopify/metafields.json') + }) + + 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) => { diff --git a/packages/theme/src/cli/services/metafields-pull.ts b/packages/theme/src/cli/services/metafields-pull.ts index f929770e601..065c0759d4d 100644 --- a/packages/theme/src/cli/services/metafields-pull.ts +++ b/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 {fileExistsSync, mkdirSync, readFileSync, writeFileSync} from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' interface MetafieldsPullOptions { @@ -161,5 +161,29 @@ 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/metafields.json') + } + + 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) + + if (gitIgnoreContent.includes(entry)) { + // The file already existing in the .gitignore + return + } + + writeFileSync(gitIgnorePath, `${gitIgnoreContent}\n${entry}`) } From 2cf54d60b3dfe7546919eef316e15c56019b38e2 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Mon, 13 Jan 2025 17:55:43 +0100 Subject: [PATCH 2/4] Use OS end-of-line markers --- packages/cli-kit/src/public/node/fs.ts | 11 +++++++++++ .../theme/src/cli/services/metafields-pull.test.ts | 9 ++++----- packages/theme/src/cli/services/metafields-pull.ts | 4 ++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index b9ee1291900..841ebe730f8 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/packages/cli-kit/src/public/node/fs.ts @@ -514,6 +514,17 @@ export async function glob(pattern: Pattern | Pattern[], options?: GlobOptions): export function pathToFileURL(path: string): URL { return pathToFile(path) } + +/** + * Returns the operating system's end-of-line marker. + * On POSIX systems this is `\n`, on Windows this is `\r\n`. + * + * @returns The platform-specific EOL string. + */ +export function eol(): string { + return os.EOL +} + /** * Find a file by walking parent directories. * diff --git a/packages/theme/src/cli/services/metafields-pull.test.ts b/packages/theme/src/cli/services/metafields-pull.test.ts index 88a93c1b7ef..6c372896b59 100644 --- a/packages/theme/src/cli/services/metafields-pull.test.ts +++ b/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, writeFileSync} from '@shopify/cli-kit/node/fs' +import {eol, fileExists, inTemporaryDirectory, readFile, writeFileSync} from '@shopify/cli-kit/node/fs' vi.mock('../utilities/theme-store.js') vi.mock('../utilities/theme-ui.js') @@ -85,9 +85,8 @@ describe('metafields-pull', () => { test('should add metafields to .gitignore', async () => { // Given - vi.mocked(metafieldDefinitionsByOwnerType).mockImplementation((type: any, _session: AdminSession) => { - if (type !== 'PRODUCT') return Promise.resolve([]) - return Promise.resolve([fakeMetafieldDefinition]) + vi.mocked(metafieldDefinitionsByOwnerType).mockImplementation(() => { + return Promise.resolve([]) }) await inTemporaryDirectory(async (tmpDir) => { @@ -100,7 +99,7 @@ describe('metafields-pull', () => { // Then await expect(fileExists(gitIgnorePath)).resolves.toBe(true) - await expect(readFile(gitIgnorePath)).resolves.toBe('.DS_Store\n.shopify/metafields.json') + await expect(readFile(gitIgnorePath)).resolves.toBe(`.DS_Store${eol()}.shopify/metafields.json`) }) expect(capturedOutput.info()).toContain('Metafield definitions have been successfully downloaded.') diff --git a/packages/theme/src/cli/services/metafields-pull.ts b/packages/theme/src/cli/services/metafields-pull.ts index 065c0759d4d..db9517f94db 100644 --- a/packages/theme/src/cli/services/metafields-pull.ts +++ b/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 {fileExistsSync, mkdirSync, readFileSync, writeFileSync} from '@shopify/cli-kit/node/fs' +import {eol, fileExistsSync, mkdirSync, readFileSync, writeFileSync} from '@shopify/cli-kit/node/fs' import {outputDebug} from '@shopify/cli-kit/node/output' interface MetafieldsPullOptions { @@ -185,5 +185,5 @@ function addToGitIgnore(root: string, entry: string) { return } - writeFileSync(gitIgnorePath, `${gitIgnoreContent}\n${entry}`) + writeFileSync(gitIgnorePath, `${gitIgnoreContent}${eol()}${entry}`) } From dcac5c66908c8cd922c7829edde0b1f45d946bdd Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 14 Jan 2025 17:08:33 +0100 Subject: [PATCH 3/4] Adjust cli-kit API --- packages/cli-kit/src/public/node/fs.test.ts | 36 +++++++++++++++++++ packages/cli-kit/src/public/node/fs.ts | 34 +++++++++++++++--- .../src/cli/services/metafields-pull.test.ts | 4 +-- .../theme/src/cli/services/metafields-pull.ts | 6 ++-- 4 files changed, 72 insertions(+), 8 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index c2382898eec..fe89483929a 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -16,6 +16,7 @@ import { generateRandomNameForSubdirectory, readFileSync, glob, + detectEOL, } from './fs.js' import {joinPath} from './path.js' import {takeRandomFromArray} from '../common/array.js' @@ -283,3 +284,38 @@ 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' + + // When + const eol = detectEOL(fileContent) + + // Then + expect(eol).toEqual('\n') + }) +}) diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index 841ebe730f8..359278ca2e9 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/packages/cli-kit/src/public/node/fs.ts @@ -515,14 +515,40 @@ 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. - * On POSIX systems this is `\n`, on Windows this is `\r\n`. * - * @returns The platform-specific EOL string. + * @returns The OS-specific end-of-line marker */ -export function eol(): string { - return os.EOL +export function defaultEOL(): EOL { + return os.EOL as EOL } /** diff --git a/packages/theme/src/cli/services/metafields-pull.test.ts b/packages/theme/src/cli/services/metafields-pull.test.ts index 6c372896b59..816e07b0c2f 100644 --- a/packages/theme/src/cli/services/metafields-pull.test.ts +++ b/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 {eol, fileExists, inTemporaryDirectory, readFile, writeFileSync} 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') @@ -99,7 +99,7 @@ describe('metafields-pull', () => { // Then await expect(fileExists(gitIgnorePath)).resolves.toBe(true) - await expect(readFile(gitIgnorePath)).resolves.toBe(`.DS_Store${eol()}.shopify/metafields.json`) + await expect(readFile(gitIgnorePath)).resolves.toBe(`.DS_Store\n.shopify/metafields.json`) }) expect(capturedOutput.info()).toContain('Metafield definitions have been successfully downloaded.') diff --git a/packages/theme/src/cli/services/metafields-pull.ts b/packages/theme/src/cli/services/metafields-pull.ts index db9517f94db..3d050f77ab8 100644 --- a/packages/theme/src/cli/services/metafields-pull.ts +++ b/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 {eol, fileExistsSync, mkdirSync, readFileSync, 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 { @@ -185,5 +185,7 @@ function addToGitIgnore(root: string, entry: string) { return } - writeFileSync(gitIgnorePath, `${gitIgnoreContent}${eol()}${entry}`) + const eol = detectEOL(gitIgnoreContent.toString()) + + writeFileSync(gitIgnorePath, `${gitIgnoreContent}${eol}${entry}`) } From bb40a595cabe1985513e34ad2900450dc49133e7 Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Tue, 14 Jan 2025 17:16:13 +0100 Subject: [PATCH 4/4] Ignore .shopify instead of .shopify/metafields.json (for consistency with the apps ecosystem) Refine .gitigore check --- packages/cli-kit/src/public/node/fs.test.ts | 3 +++ packages/theme/src/cli/services/metafields-pull.test.ts | 4 ++-- packages/theme/src/cli/services/metafields-pull.ts | 9 ++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index fe89483929a..5db3ac83c08 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/packages/cli-kit/src/public/node/fs.test.ts @@ -22,9 +22,11 @@ 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 () => { @@ -311,6 +313,7 @@ describe('detectEOL', () => { 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) diff --git a/packages/theme/src/cli/services/metafields-pull.test.ts b/packages/theme/src/cli/services/metafields-pull.test.ts index 816e07b0c2f..603cf9468d4 100644 --- a/packages/theme/src/cli/services/metafields-pull.test.ts +++ b/packages/theme/src/cli/services/metafields-pull.test.ts @@ -92,14 +92,14 @@ describe('metafields-pull', () => { await inTemporaryDirectory(async (tmpDir) => { // Given const gitIgnorePath = `${tmpDir}/.gitignore` - writeFileSync(gitIgnorePath, '.DS_Store') + 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/metafields.json`) + await expect(readFile(gitIgnorePath)).resolves.toBe(`.DS_Store\n.shopify/secrets.json\n.shopify`) }) expect(capturedOutput.info()).toContain('Metafield definitions have been successfully downloaded.') diff --git a/packages/theme/src/cli/services/metafields-pull.ts b/packages/theme/src/cli/services/metafields-pull.ts index 3d050f77ab8..b85eb7f6c0e 100644 --- a/packages/theme/src/cli/services/metafields-pull.ts +++ b/packages/theme/src/cli/services/metafields-pull.ts @@ -164,7 +164,7 @@ function writeMetafieldDefinitionsToFile(path: string, content: unknown) { const fileContent = JSON.stringify(content, null, 2) if (!fileExistsSync(filePath)) { - addToGitIgnore(path, '.shopify/metafields.json') + addToGitIgnore(path, '.shopify') } writeFileSync(filePath, fileContent) @@ -178,14 +178,13 @@ function addToGitIgnore(root: string, entry: string) { return } - const gitIgnoreContent = readFileSync(gitIgnorePath) + const gitIgnoreContent = readFileSync(gitIgnorePath).toString() + const eol = detectEOL(gitIgnoreContent) - if (gitIgnoreContent.includes(entry)) { + if (gitIgnoreContent.split(eol).includes(entry)) { // The file already existing in the .gitignore return } - const eol = detectEOL(gitIgnoreContent.toString()) - writeFileSync(gitIgnorePath, `${gitIgnoreContent}${eol}${entry}`) }