From 09881b1e00e3ab1ef7b1a0d971411b198ec1f77e Mon Sep 17 00:00:00 2001 From: Alok Swamy Date: Fri, 2 Jan 2026 16:15:26 -0500 Subject: [PATCH] Notify users when Liquid files are rewritten remotely --- .changeset/sour-boats-grab.md | 6 + .../admin/generated/theme_files_upsert.ts | 3 +- .../mutations/theme_files_upsert.graphql | 1 + .../src/public/node/themes/api.test.ts | 8 ++ .../cli-kit/src/public/node/themes/api.ts | 4 + packages/theme/src/cli/services/dev.ts | 6 +- packages/theme/src/cli/services/push.test.ts | 2 + packages/theme/src/cli/services/push.ts | 8 +- .../theme/src/cli/utilities/asset-checksum.ts | 6 + .../src/cli/utilities/theme-downloader.ts | 7 +- .../theme-environment.test.ts | 9 +- .../theme-environment/theme-environment.ts | 3 + .../theme/src/cli/utilities/theme-fs.test.ts | 110 +++++++++++----- packages/theme/src/cli/utilities/theme-fs.ts | 68 +++++++--- .../src/cli/utilities/theme-uploader.test.ts | 117 +++++++++++++++++- .../theme/src/cli/utilities/theme-uploader.ts | 74 ++++++++++- 16 files changed, 367 insertions(+), 65 deletions(-) create mode 100644 .changeset/sour-boats-grab.md diff --git a/.changeset/sour-boats-grab.md b/.changeset/sour-boats-grab.md new file mode 100644 index 00000000000..80df2c2053c --- /dev/null +++ b/.changeset/sour-boats-grab.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': minor +'@shopify/theme': minor +--- + +Add ability to notify user when Liquid files are rewritten remotely diff --git a/packages/cli-kit/src/cli/api/graphql/admin/generated/theme_files_upsert.ts b/packages/cli-kit/src/cli/api/graphql/admin/generated/theme_files_upsert.ts index f7bd095c8f5..88bacb36ff7 100644 --- a/packages/cli-kit/src/cli/api/graphql/admin/generated/theme_files_upsert.ts +++ b/packages/cli-kit/src/cli/api/graphql/admin/generated/theme_files_upsert.ts @@ -10,7 +10,7 @@ export type ThemeFilesUpsertMutationVariables = Types.Exact<{ export type ThemeFilesUpsertMutation = { themeFilesUpsert?: { - upsertedThemeFiles?: {filename: string}[] | null + upsertedThemeFiles?: {filename: string; checksumMd5?: string | null}[] | null userErrors: {filename?: string | null; message: string}[] } | null } @@ -71,6 +71,7 @@ export const ThemeFilesUpsert = { kind: 'SelectionSet', selections: [ {kind: 'Field', name: {kind: 'Name', value: 'filename'}}, + {kind: 'Field', name: {kind: 'Name', value: 'checksumMd5'}}, {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, ], }, diff --git a/packages/cli-kit/src/cli/api/graphql/admin/mutations/theme_files_upsert.graphql b/packages/cli-kit/src/cli/api/graphql/admin/mutations/theme_files_upsert.graphql index 1106e4addc2..f0916b89b36 100644 --- a/packages/cli-kit/src/cli/api/graphql/admin/mutations/theme_files_upsert.graphql +++ b/packages/cli-kit/src/cli/api/graphql/admin/mutations/theme_files_upsert.graphql @@ -2,6 +2,7 @@ mutation themeFilesUpsert($files: [OnlineStoreThemeFilesUpsertFileInput!]!, $the themeFilesUpsert(files: $files, themeId: $themeId) { upsertedThemeFiles { filename + checksumMd5 } userErrors { filename diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index d3dc4e06736..f2d97c6ee43 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -553,11 +553,19 @@ describe('bulkUploadThemeAssets', async () => { key: 'snippets/product-variant-picker.liquid', success: true, operation: Operation.Upload, + asset: { + checksum: '', + key: 'snippets/product-variant-picker.liquid', + }, }, { key: 'templates/404.json', success: true, operation: Operation.Upload, + asset: { + checksum: '', + key: 'templates/404.json', + }, }, ]) }) diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index 2af4ad3295b..4f5ab4a1e90 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -316,6 +316,10 @@ function processUploadResults(uploadResults: ThemeFilesUpsertMutation): Result[] key: file.filename, success: true, operation: Operation.Upload, + asset: { + key: file.filename, + checksum: file.checksumMd5 ?? '', + }, }) }) diff --git a/packages/theme/src/cli/services/dev.ts b/packages/theme/src/cli/services/dev.ts index a54ef69fcf8..439c3226a3c 100644 --- a/packages/theme/src/cli/services/dev.ts +++ b/packages/theme/src/cli/services/dev.ts @@ -120,7 +120,10 @@ export async function dev(options: DevOptions) { }, } - const {serverStart, renderDevSetupProgress, backgroundJobPromise} = setupDevServer(options.theme, ctx) + const {serverStart, renderDevSetupProgress, backgroundJobPromise, syncRewrittenFilesPromise} = setupDevServer( + options.theme, + ctx, + ) readline.emitKeypressEvents(process.stdin) if (process.stdin.isTTY) { @@ -140,6 +143,7 @@ export async function dev(options: DevOptions) { openURLSafely(urls.local, 'development server') } }), + syncRewrittenFilesPromise(), ]) } diff --git a/packages/theme/src/cli/services/push.test.ts b/packages/theme/src/cli/services/push.test.ts index 9cb40923822..4754739f92c 100644 --- a/packages/theme/src/cli/services/push.test.ts +++ b/packages/theme/src/cli/services/push.test.ts @@ -49,6 +49,7 @@ describe('push', () => { workPromise: Promise.resolve(), uploadResults: new Map(), renderThemeSyncProgress: () => Promise.resolve(), + syncRewrittenFilesPromise: Promise.resolve(), }) vi.mocked(ensureThemeStore).mockReturnValue('example.myshopify.com') vi.mocked(ensureAuthenticatedThemes).mockResolvedValue(adminSession) @@ -93,6 +94,7 @@ describe('push', () => { workPromise: Promise.resolve(), uploadResults, renderThemeSyncProgress: () => Promise.resolve(), + syncRewrittenFilesPromise: Promise.resolve(), }) // When diff --git a/packages/theme/src/cli/services/push.ts b/packages/theme/src/cli/services/push.ts index 1015a5af0ac..9858cbb0c43 100644 --- a/packages/theme/src/cli/services/push.ts +++ b/packages/theme/src/cli/services/push.ts @@ -205,16 +205,20 @@ async function executePush( const themeFileSystem = mountThemeFileSystem(options.path, {filters: options}) recordTiming('theme-service:push:file-system') - const {uploadResults, renderThemeSyncProgress} = uploadTheme( + const {uploadResults, renderThemeSyncProgress, syncRewrittenFilesPromise} = uploadTheme( theme, session, themeChecksums, themeFileSystem, - options, + { + ...options, + handleRewrittenFiles: 'warn', + }, context, ) await renderThemeSyncProgress() + await syncRewrittenFilesPromise if (options.publish) { await themePublish(theme.id, session) diff --git a/packages/theme/src/cli/utilities/asset-checksum.ts b/packages/theme/src/cli/utilities/asset-checksum.ts index e80a5f86257..87d1c985d06 100644 --- a/packages/theme/src/cli/utilities/asset-checksum.ts +++ b/packages/theme/src/cli/utilities/asset-checksum.ts @@ -35,10 +35,16 @@ function minifiedJSONFileChecksum(fileContent: string) { function regularFileChecksum(fileKey: string, fileContent: string) { let content = fileContent + // eslint-disable-next-line no-console + console.log('MD5 before replace newlines:', md5(content)) + if (isTextFile(fileKey)) { content = content.replace(/\r\n/g, '\n') } + // eslint-disable-next-line no-console + console.log('MD5 after replace newlines:', md5(content)) + return md5(content) } diff --git a/packages/theme/src/cli/utilities/theme-downloader.ts b/packages/theme/src/cli/utilities/theme-downloader.ts index fae80deb311..beecaff489a 100644 --- a/packages/theme/src/cli/utilities/theme-downloader.ts +++ b/packages/theme/src/cli/utilities/theme-downloader.ts @@ -86,7 +86,12 @@ function buildDownloadTasks( return batches } -async function downloadFiles(theme: Theme, fileSystem: ThemeFileSystem, filenames: string[], session: AdminSession) { +export async function downloadFiles( + theme: Theme, + fileSystem: ThemeFileSystem, + filenames: string[], + session: AdminSession, +) { const assets = await fetchThemeAssets(theme.id, filenames, session) if (!assets) return diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index cd2139c5abd..9476f708625 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -35,7 +35,12 @@ vi.mock('../theme-uploader.js', async () => { }) beforeEach(() => { vi.mocked(uploadTheme).mockImplementation(() => { - return {workPromise: Promise.resolve(), uploadResults: new Map(), renderThemeSyncProgress: () => Promise.resolve()} + return { + workPromise: Promise.resolve(), + uploadResults: new Map(), + renderThemeSyncProgress: () => Promise.resolve(), + syncRewrittenFilesPromise: Promise.resolve(), + } }) }) @@ -127,6 +132,7 @@ describe('setupDevServer', () => { nodelete: true, deferPartialWork: true, backgroundWorkCatch: expect.any(Function), + handleRewrittenFiles: 'fix', }) }) @@ -185,6 +191,7 @@ describe('setupDevServer', () => { nodelete: true, deferPartialWork: true, backgroundWorkCatch: expect.any(Function), + handleRewrittenFiles: 'fix', }) }) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts index 34bbd14d710..daafcc3b260 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts @@ -29,6 +29,7 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) { dispatchEvent: server.dispatch, renderDevSetupProgress: envSetup.renderProgress, backgroundJobPromise, + syncRewrittenFilesPromise: () => envSetup.syncRewrittenFilesPromise, } } @@ -58,6 +59,7 @@ function ensureThemeEnvironmentSetup( nodelete: ctx.options.noDelete, deferPartialWork: true, backgroundWorkCatch: abort, + handleRewrittenFiles: 'fix', }) }) @@ -82,6 +84,7 @@ function ensureThemeEnvironmentSetup( await renderThemeSyncProgress() }, + syncRewrittenFilesPromise: uploadPromise.then((result) => result.syncRewrittenFilesPromise).catch(abort), } } diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts index 62236e64dba..5b8cf902ec2 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -39,6 +39,12 @@ beforeEach(async () => { vi.mocked(applyIgnoreFilters).mockImplementation(realModule.applyIgnoreFilters) }) +interface FileContent { + value?: string + attachment?: string + checksum: string +} + describe('theme-fs', () => { const locationOfThisFile = dirname(fileURLToPath(import.meta.url)) @@ -700,6 +706,7 @@ describe('theme-fs', () => { const adminSession = {token: 'token'} as AdminSession let unsyncedFileKeys: Set let uploadErrors: Map + const write = vi.fn() beforeEach(() => { unsyncedFileKeys = new Set([fileKey]) @@ -710,10 +717,10 @@ describe('theme-fs', () => { test('returns false if file is not in unsyncedFileKeys', async () => { // Given unsyncedFileKeys = new Set() - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) // When - const result = await handler({value: 'content'}) + const result = await handler({value: 'content', checksum: '123'}) // Then expect(result).toBe(false) @@ -721,34 +728,68 @@ describe('theme-fs', () => { expect(triggerBrowserFullReload).not.toHaveBeenCalled() }) - Object.entries({ - text: {value: 'content'}, - image: {attachment: 'content'}, - }).forEach(([fileType, fileContent]) => { - test(`uploads ${fileType} file and returns true on successful sync`, async () => { - // Given - vi.mocked(bulkUploadThemeAssets).mockResolvedValue([ - { - key: fileKey, - success: true, - operation: Operation.Upload, - }, - ]) - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) - - // When - const result = await handler(fileContent) - - // Then - expect(result).toBe(true) - expect(bulkUploadThemeAssets).toHaveBeenCalledWith( - Number(themeId), - [{key: fileKey, ...fileContent}], - adminSession, - ) - expect(unsyncedFileKeys.has(fileKey)).toBe(false) - expect(triggerBrowserFullReload).not.toHaveBeenCalled() - }) + test.each<[string, FileContent]>([ + ['text', {value: 'content', checksum: '123'}], + ['image', {attachment: 'content', checksum: '123'}], + ])('uploads %s file and returns true on successful sync', async (_fileType, fileContent) => { + // Given + vi.mocked(bulkUploadThemeAssets).mockResolvedValue([ + { + key: fileKey, + success: true, + operation: Operation.Upload, + }, + ]) + vi.mocked(fetchThemeAssets).mockResolvedValue([]) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) + + // When + const result = await handler(fileContent) + + // Then + expect(result).toBe(true) + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + Number(themeId), + [{key: fileKey, ...fileContent}], + adminSession, + ) + expect(unsyncedFileKeys.has(fileKey)).toBe(false) + expect(triggerBrowserFullReload).not.toHaveBeenCalled() + }) + + test('updates the file locally if an unsyncedFile is pushed up but rewritten remotely', async () => { + const liquidFileKey = 'snippets/new-file.liquid' + const originalFileContent = { + key: liquidFileKey, + checksum: 'checksum', + value: 'content', + } + const rewrittenFileContent = { + key: liquidFileKey, + checksum: 'rewritten-checksum', + value: 'rewritten content', + } + unsyncedFileKeys = new Set([liquidFileKey]) + // Given + vi.mocked(bulkUploadThemeAssets).mockResolvedValue([ + { + key: liquidFileKey, + success: true, + operation: Operation.Upload, + asset: rewrittenFileContent, + }, + ]) + vi.mocked(fetchThemeAssets).mockResolvedValue([rewrittenFileContent]) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, liquidFileKey, themeId, adminSession, write) + + // When + const result = await handler({value: originalFileContent.value, checksum: originalFileContent.checksum}) + + // Then + expect(result).toBe(true) + expect(bulkUploadThemeAssets).toHaveBeenCalledWith(Number(themeId), [originalFileContent], adminSession) + expect(unsyncedFileKeys.has(liquidFileKey)).toBe(false) + expect(write).toHaveBeenCalledWith(rewrittenFileContent) }) test('throws error and sets uploadErrors on failed sync', async () => { @@ -762,10 +803,10 @@ describe('theme-fs', () => { errors: {asset: errors}, }, ]) - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) // When/Then - await expect(handler({value: 'content'})).rejects.toThrow('{{ broken liquid file') + await expect(handler({value: 'content', checksum: '123'})).rejects.toThrow('{{ broken liquid file') expect(uploadErrors.get(fileKey)).toEqual(errors) expect(unsyncedFileKeys.has(fileKey)).toBe(true) expect(triggerBrowserFullReload).toHaveBeenCalledWith(themeId, fileKey) @@ -781,10 +822,11 @@ describe('theme-fs', () => { operation: Operation.Upload, }, ]) - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) + vi.mocked(fetchThemeAssets).mockResolvedValue([]) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) // When - await handler({value: 'content'}) + await handler({value: 'content', checksum: '123'}) // Then expect(uploadErrors.has(fileKey)).toBe(false) diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts index 4d078849552..866bb8f5e1f 100644 --- a/packages/theme/src/cli/utilities/theme-fs.ts +++ b/packages/theme/src/cli/utilities/theme-fs.ts @@ -7,10 +7,17 @@ import {DEFAULT_IGNORE_PATTERNS, timestampDateFormat} from '../constants.js' import {glob, readFile, ReadOptions, fileExists, mkdir, writeFile, removeFile} from '@shopify/cli-kit/node/fs' import {joinPath, basename, relativePath} from '@shopify/cli-kit/node/path' import {lookupMimeType, setMimeTypes} from '@shopify/cli-kit/node/mimes' -import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} from '@shopify/cli-kit/node/output' +import { + outputContent, + outputDebug, + outputInfo, + outputToken, + outputWarn, + TokenizedString, +} from '@shopify/cli-kit/node/output' import {buildThemeAsset} from '@shopify/cli-kit/node/themes/factories' import {AdminSession} from '@shopify/cli-kit/node/session' -import {bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api' +import {bulkUploadThemeAssets, deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api' import EventEmitter from 'node:events' import {fileURLToPath} from 'node:url' import type { @@ -125,6 +132,19 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti return notifier?.notify(fileKey) ?? Promise.resolve() } + const write = async (asset: ThemeAsset) => { + files.set( + asset.key, + buildThemeAsset({ + key: asset.key, + checksum: asset.checksum, + value: asset.value ?? '', + attachment: asset.attachment ?? '', + }), + ) + await writeThemeFile(root, asset) + } + const handleFileUpdate = ( eventName: 'add' | 'change', themeId: string, @@ -152,6 +172,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing value: file.value || undefined, attachment: file.attachment, + checksum: file.checksum, } }) @@ -159,7 +180,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti .then((content) => { if (!content) return - return handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession)(content) + return handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write)(content) }) .catch(createSyncingCatchError(fileKey, 'upload')) @@ -242,18 +263,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti files.delete(fileKey) await removeThemeFile(root, fileKey) }, - write: async (asset: ThemeAsset) => { - files.set( - asset.key, - buildThemeAsset({ - key: asset.key, - checksum: asset.checksum, - value: asset.value ?? '', - attachment: asset.attachment ?? '', - }), - ) - await writeThemeFile(root, asset) - }, + write, read, applyIgnoreFilters: (files) => applyIgnoreFilters(files, filterPatterns), addEventListener: (eventName, cb) => { @@ -282,7 +292,8 @@ export function handleSyncUpdate( fileKey: string, themeId: string, adminSession: AdminSession, -): (content: {value?: string; attachment?: string}) => PromiseLike { + write: (asset: ThemeAsset) => Promise, +): (content: {value?: string; attachment?: string; checksum: string}) => Promise { return async (content) => { if (!unsyncedFileKeys.has(fileKey)) { return false @@ -306,6 +317,15 @@ export function handleSyncUpdate( unsyncedFileKeys.delete(fileKey) outputSyncResult('update', fileKey) + if (content.value && content.checksum !== result?.asset?.checksum) { + const [remoteAsset] = await fetchThemeAssets(Number(themeId), [fileKey], adminSession) + + if (remoteAsset) { + await write(remoteAsset) + outputSyncResult('rewrite', fileKey) + } + } + return true } } @@ -462,10 +482,18 @@ function dirPath(filePath: string) { return filePath.substring(0, fileNameIndex) } -function outputSyncResult(action: 'update' | 'delete', fileKey: string): void { - outputInfo( - outputContent`• ${timestampDateFormat.format(new Date())} Synced ${outputToken.raw('»')} ${action} ${fileKey}`, - ) +function outputSyncResult(action: 'update' | 'delete' | 'rewrite', fileKey: string): void { + let content: TokenizedString + + if (action === 'rewrite') { + content = outputContent`• ${timestampDateFormat.format(new Date())} Overwritten ${fileKey}` + } else { + content = outputContent`• ${timestampDateFormat.format(new Date())} Synced ${outputToken.raw( + '»', + )} ${action} ${fileKey}` + } + + outputInfo(content) } export function inferLocalHotReloadScriptPath() { diff --git a/packages/theme/src/cli/utilities/theme-uploader.test.ts b/packages/theme/src/cli/utilities/theme-uploader.test.ts index 5b5b3fae062..1aa10b4d30a 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.test.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.test.ts @@ -8,10 +8,11 @@ import { } from './theme-uploader.js' import {fakeThemeFileSystem} from './theme-fs/theme-fs-mock-factory.js' import {renderTasksToStdErr} from './theme-ui.js' -import {bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api' +import {bulkUploadThemeAssets, deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api' import {Result, Checksum, Key, ThemeAsset, Operation} from '@shopify/cli-kit/node/themes/types' import {beforeEach, describe, expect, test, vi} from 'vitest' import {AdminSession} from '@shopify/cli-kit/node/session' +import {renderInfo} from '@shopify/cli-kit/node/ui' vi.mock('@shopify/cli-kit/node/themes/api') vi.mock('@shopify/cli-kit/node/ui') @@ -246,6 +247,120 @@ describe('theme-uploader', () => { ) }) + test('should notify when Liquid files are rewritten remotely when handleRewrittenFiles="warn"', async () => { + // Given + const remoteChecksums = [{key: 'assets/matching.liquid', checksum: '1', value: 'content'}] + const themeFileSystem = fakeThemeFileSystem( + 'tmp', + new Map([ + ['assets/matching.liquid', {checksum: '1', key: '', value: 'content'}], + ['snippets/new-file.liquid', {checksum: '2', key: '', value: 'content'}], + ]), + ) + + // When + const {renderThemeSyncProgress, syncRewrittenFilesPromise} = uploadTheme( + remoteTheme, + adminSession, + remoteChecksums, + themeFileSystem, + { + ...uploadOptions, + handleRewrittenFiles: 'warn', + }, + ) + await renderThemeSyncProgress() + await syncRewrittenFilesPromise + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(2) + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: 'snippets/new-file.liquid', + value: 'content', + }, + ], + adminSession, + ) + expect(renderInfo).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.arrayContaining([ + expect.objectContaining({ + list: { + items: ['snippets/new-file.liquid'], + }, + }), + ]), + }), + ) + }) + + test('should fetch and persist Liquid files when they are rewritten remotely when handleRewrittenFiles="fix"', async () => { + const rewrittenLiquidFileAsset = { + key: 'snippets/new-file.liquid', + checksum: 'rewritten-checksum', + value: 'rewritten content', + } as const + + vi.mocked(fetchThemeAssets).mockImplementation((_id, filenames, _session) => { + if (filenames.includes(rewrittenLiquidFileAsset.key)) { + return Promise.resolve([rewrittenLiquidFileAsset]) + } + return Promise.resolve([]) + }) + + // Given + const remoteChecksums = [{key: 'assets/matching.liquid', checksum: '1', value: 'content'}] + const themeFileSystem = fakeThemeFileSystem( + 'tmp', + new Map([ + ['assets/matching.liquid', {checksum: '1', key: '', value: 'content'}], + ['snippets/new-file.liquid', {checksum: '2', key: '', value: 'content'}], + ]), + ) + + // When + const {renderThemeSyncProgress, syncRewrittenFilesPromise} = uploadTheme( + remoteTheme, + adminSession, + remoteChecksums, + themeFileSystem, + { + ...uploadOptions, + handleRewrittenFiles: 'fix', + }, + ) + await renderThemeSyncProgress() + await syncRewrittenFilesPromise + + // Then + expect(bulkUploadThemeAssets).toHaveBeenCalledTimes(2) + expect(bulkUploadThemeAssets).toHaveBeenCalledWith( + remoteTheme.id, + [ + { + key: rewrittenLiquidFileAsset.key, + value: 'content', + }, + ], + adminSession, + ) + expect(renderInfo).toHaveBeenCalledWith( + expect.objectContaining({ + body: expect.arrayContaining([ + expect.objectContaining({ + list: { + items: [rewrittenLiquidFileAsset.key], + }, + }), + ]), + }), + ) + expect(themeFileSystem.files.get(rewrittenLiquidFileAsset.key)).toEqual(rewrittenLiquidFileAsset) + }) + test('should delete files in correct order', async () => { // Given const remoteChecksums: Checksum[] = [ diff --git a/packages/theme/src/cli/utilities/theme-uploader.ts b/packages/theme/src/cli/utilities/theme-uploader.ts index aa860ef5c3e..45df0f19ac2 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.ts @@ -3,10 +3,11 @@ import {rejectGeneratedStaticAssets} from './asset-checksum.js' import {createSyncingCatchError, renderThrownError} from './errors.js' import {triggerBrowserFullReload} from './theme-environment/hot-reload/server.js' import {renderTasksToStdErr} from './theme-ui.js' +import {downloadFiles} from './theme-downloader.js' import {AdminSession} from '@shopify/cli-kit/node/session' import {Result, Checksum, Theme, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types' import {AssetParams, bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api' -import {Task} from '@shopify/cli-kit/node/ui' +import {renderInfo, Task} from '@shopify/cli-kit/node/ui' import {outputDebug} from '@shopify/cli-kit/node/output' import {recordEvent} from '@shopify/cli-kit/node/analytics' import {Writable} from 'stream' @@ -17,6 +18,7 @@ interface UploadOptions { backgroundWorkCatch?: (error: Error) => never environment?: string multiEnvironment?: boolean + handleRewrittenFiles?: 'warn' | 'fix' } type ChecksumWithSize = Checksum & {size: number} @@ -60,7 +62,7 @@ export function uploadTheme( .then(() => buildDeleteJob(remoteChecksums, themeFileSystem, theme, session, options, uploadResults)) const workPromise = options?.deferPartialWork - ? themeCreationPromise + ? themeCreationPromise.then(() => {}) : deleteJobPromise.then((result) => result.promise) if (options?.backgroundWorkCatch) { @@ -72,9 +74,71 @@ export function uploadTheme( ]).catch(options.backgroundWorkCatch) } + const syncRewrittenFilesPromise = options?.handleRewrittenFiles + ? Promise.all([themeFileSystem.ready(), themeCreationPromise, uploadJobPromise, deleteJobPromise]).then( + async () => { + const filesDifferentFromRemote: string[] = [] + + for (const uploadResult of uploadResults.values()) { + if (!uploadResult.key.endsWith('.liquid')) continue + + const localFile = themeFileSystem.files.get(uploadResult.key) + + if ( + localFile?.value && + uploadResult.success && + uploadResult.asset?.checksum && + uploadResult.asset.checksum !== localFile.checksum + ) { + filesDifferentFromRemote.push(uploadResult.key) + } + } + + if (filesDifferentFromRemote.length > 0) { + if (options.handleRewrittenFiles === 'fix') { + renderInfo({ + headline: `Updated Liquid files to conform to latest Liquid standards.`, + body: [ + `The following Liquid files were updated locally and remotely:`, + { + list: { + items: filesDifferentFromRemote, + }, + }, + ], + }) + await downloadFiles(theme, themeFileSystem, filesDifferentFromRemote, session) + } else if (options.handleRewrittenFiles === 'warn') { + renderInfo({ + headline: `Liquid files were updated remotely to conform to latest Liquid standards.`, + body: [ + 'The following Liquid files were updated remotely:', + { + list: { + items: filesDifferentFromRemote, + }, + }, + ], + nextSteps: [ + [ + 'Fetch the latest files using', + { + command: 'shopify theme pull', + }, + 'to sync them locally.', + ], + ], + }) + } + } + }, + ) + : Promise.resolve() + return { uploadResults, workPromise, + syncRewrittenFilesPromise, renderThemeSyncProgress: async () => { if (options?.deferPartialWork) return @@ -237,9 +301,11 @@ async function ensureThemeCreation(theme: Theme, session: AdminSession, remoteCh const remoteAssetKeys = new Set(remoteChecksums.map((checksum) => checksum.key)) const missingAssets = MINIMUM_THEME_ASSETS.filter(({key}) => !remoteAssetKeys.has(key)) - if (missingAssets.length > 0) { - await bulkUploadThemeAssets(theme.id, missingAssets, session) + if (missingAssets.length === 0) { + return Promise.resolve([]) } + + return bulkUploadThemeAssets(theme.id, missingAssets, session) } interface SyncJob {