diff --git a/.changeset/honest-corners-relate.md b/.changeset/honest-corners-relate.md new file mode 100644 index 00000000000..7f038e37fcc --- /dev/null +++ b/.changeset/honest-corners-relate.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/theme': patch +--- + +[Bug fix] Remove ability to sync Liquid files by checksum 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 88bacb36ff7..f7bd095c8f5 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; checksumMd5?: string | null}[] | null + upsertedThemeFiles?: {filename: string}[] | null userErrors: {filename?: string | null; message: string}[] } | null } @@ -71,7 +71,6 @@ 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 f0916b89b36..1106e4addc2 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,7 +2,6 @@ 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 f2d97c6ee43..d3dc4e06736 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -553,19 +553,11 @@ 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 8746fa9e7b4..fccd66b5614 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -315,10 +315,6 @@ 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 fea7b768269..89742bf6932 100644 --- a/packages/theme/src/cli/services/dev.ts +++ b/packages/theme/src/cli/services/dev.ts @@ -101,7 +101,7 @@ export async function dev(options: DevOptions) { session.storefrontPassword = await storefrontPasswordPromise } - const {serverStart, renderDevSetupProgress, syncRewrittenFilesPromise} = setupDevServer(options.theme, ctx) + const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx) if (!options['theme-editor-sync']) { session.storefrontPassword = await storefrontPasswordPromise @@ -109,7 +109,6 @@ export async function dev(options: DevOptions) { await renderDevSetupProgress() await serverStart() - await syncRewrittenFilesPromise() renderLinks(urls) if (options.open) { diff --git a/packages/theme/src/cli/services/push.test.ts b/packages/theme/src/cli/services/push.test.ts index 4754739f92c..9cb40923822 100644 --- a/packages/theme/src/cli/services/push.test.ts +++ b/packages/theme/src/cli/services/push.test.ts @@ -49,7 +49,6 @@ 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) @@ -94,7 +93,6 @@ 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 9858cbb0c43..1015a5af0ac 100644 --- a/packages/theme/src/cli/services/push.ts +++ b/packages/theme/src/cli/services/push.ts @@ -205,20 +205,16 @@ async function executePush( const themeFileSystem = mountThemeFileSystem(options.path, {filters: options}) recordTiming('theme-service:push:file-system') - const {uploadResults, renderThemeSyncProgress, syncRewrittenFilesPromise} = uploadTheme( + const {uploadResults, renderThemeSyncProgress} = uploadTheme( theme, session, themeChecksums, themeFileSystem, - { - ...options, - handleRewrittenFiles: 'warn', - }, + options, context, ) await renderThemeSyncProgress() - await syncRewrittenFilesPromise if (options.publish) { await themePublish(theme.id, session) diff --git a/packages/theme/src/cli/utilities/theme-downloader.ts b/packages/theme/src/cli/utilities/theme-downloader.ts index beecaff489a..fae80deb311 100644 --- a/packages/theme/src/cli/utilities/theme-downloader.ts +++ b/packages/theme/src/cli/utilities/theme-downloader.ts @@ -86,12 +86,7 @@ function buildDownloadTasks( return batches } -export async function downloadFiles( - theme: Theme, - fileSystem: ThemeFileSystem, - filenames: string[], - session: AdminSession, -) { +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 55080ea582f..9cc63555baf 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 @@ -34,12 +34,7 @@ vi.mock('../theme-uploader.js', async () => { }) beforeEach(() => { vi.mocked(uploadTheme).mockImplementation(() => { - return { - workPromise: Promise.resolve(), - uploadResults: new Map(), - renderThemeSyncProgress: () => Promise.resolve(), - syncRewrittenFilesPromise: Promise.resolve(), - } + return {workPromise: Promise.resolve(), uploadResults: new Map(), renderThemeSyncProgress: () => Promise.resolve()} }) }) @@ -131,7 +126,6 @@ describe('setupDevServer', () => { nodelete: true, deferPartialWork: true, backgroundWorkCatch: expect.any(Function), - handleRewrittenFiles: 'fix', }) }) @@ -182,7 +176,6 @@ 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 e7f87135c08..9c35377cd68 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts @@ -25,7 +25,6 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) { serverStart: server.start, dispatchEvent: server.dispatch, renderDevSetupProgress: envSetup.renderProgress, - syncRewrittenFilesPromise: () => envSetup.syncRewrittenFilesPromise, } } @@ -45,7 +44,6 @@ function ensureThemeEnvironmentSetup(theme: Theme, ctx: DevServerContext) { nodelete: ctx.options.noDelete, deferPartialWork: true, backgroundWorkCatch: abort, - handleRewrittenFiles: 'fix', }) }) .catch(abort) @@ -69,7 +67,6 @@ function ensureThemeEnvironmentSetup(theme: Theme, ctx: DevServerContext) { 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 5b8cf902ec2..62236e64dba 100644 --- a/packages/theme/src/cli/utilities/theme-fs.test.ts +++ b/packages/theme/src/cli/utilities/theme-fs.test.ts @@ -39,12 +39,6 @@ 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)) @@ -706,7 +700,6 @@ describe('theme-fs', () => { const adminSession = {token: 'token'} as AdminSession let unsyncedFileKeys: Set let uploadErrors: Map - const write = vi.fn() beforeEach(() => { unsyncedFileKeys = new Set([fileKey]) @@ -717,10 +710,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, write) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) // When - const result = await handler({value: 'content', checksum: '123'}) + const result = await handler({value: 'content'}) // Then expect(result).toBe(false) @@ -728,68 +721,34 @@ describe('theme-fs', () => { 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) + 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('throws error and sets uploadErrors on failed sync', async () => { @@ -803,10 +762,10 @@ describe('theme-fs', () => { errors: {asset: errors}, }, ]) - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) // When/Then - await expect(handler({value: 'content', checksum: '123'})).rejects.toThrow('{{ broken liquid file') + await expect(handler({value: 'content'})).rejects.toThrow('{{ broken liquid file') expect(uploadErrors.get(fileKey)).toEqual(errors) expect(unsyncedFileKeys.has(fileKey)).toBe(true) expect(triggerBrowserFullReload).toHaveBeenCalledWith(themeId, fileKey) @@ -822,11 +781,10 @@ describe('theme-fs', () => { operation: Operation.Upload, }, ]) - vi.mocked(fetchThemeAssets).mockResolvedValue([]) - const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write) + const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession) // When - await handler({value: 'content', checksum: '123'}) + await handler({value: 'content'}) // 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 866bb8f5e1f..4d078849552 100644 --- a/packages/theme/src/cli/utilities/theme-fs.ts +++ b/packages/theme/src/cli/utilities/theme-fs.ts @@ -7,17 +7,10 @@ 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, - TokenizedString, -} from '@shopify/cli-kit/node/output' +import {outputContent, outputDebug, outputInfo, outputToken, outputWarn} 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, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api' +import {bulkUploadThemeAssets, deleteThemeAssets} from '@shopify/cli-kit/node/themes/api' import EventEmitter from 'node:events' import {fileURLToPath} from 'node:url' import type { @@ -132,19 +125,6 @@ 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, @@ -172,7 +152,6 @@ 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, } }) @@ -180,7 +159,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti .then((content) => { if (!content) return - return handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession, write)(content) + return handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession)(content) }) .catch(createSyncingCatchError(fileKey, 'upload')) @@ -263,7 +242,18 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti files.delete(fileKey) await removeThemeFile(root, fileKey) }, - write, + 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) + }, read, applyIgnoreFilters: (files) => applyIgnoreFilters(files, filterPatterns), addEventListener: (eventName, cb) => { @@ -292,8 +282,7 @@ export function handleSyncUpdate( fileKey: string, themeId: string, adminSession: AdminSession, - write: (asset: ThemeAsset) => Promise, -): (content: {value?: string; attachment?: string; checksum: string}) => Promise { +): (content: {value?: string; attachment?: string}) => PromiseLike { return async (content) => { if (!unsyncedFileKeys.has(fileKey)) { return false @@ -317,15 +306,6 @@ 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 } } @@ -482,18 +462,10 @@ function dirPath(filePath: string) { return filePath.substring(0, fileNameIndex) } -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) +function outputSyncResult(action: 'update' | 'delete', fileKey: string): void { + outputInfo( + outputContent`• ${timestampDateFormat.format(new Date())} Synced ${outputToken.raw('»')} ${action} ${fileKey}`, + ) } 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 1aa10b4d30a..5b5b3fae062 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.test.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.test.ts @@ -8,11 +8,10 @@ import { } from './theme-uploader.js' import {fakeThemeFileSystem} from './theme-fs/theme-fs-mock-factory.js' import {renderTasksToStdErr} from './theme-ui.js' -import {bulkUploadThemeAssets, deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api' +import {bulkUploadThemeAssets, deleteThemeAssets} 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') @@ -247,120 +246,6 @@ 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 45df0f19ac2..aa860ef5c3e 100644 --- a/packages/theme/src/cli/utilities/theme-uploader.ts +++ b/packages/theme/src/cli/utilities/theme-uploader.ts @@ -3,11 +3,10 @@ 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 {renderInfo, Task} from '@shopify/cli-kit/node/ui' +import {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' @@ -18,7 +17,6 @@ interface UploadOptions { backgroundWorkCatch?: (error: Error) => never environment?: string multiEnvironment?: boolean - handleRewrittenFiles?: 'warn' | 'fix' } type ChecksumWithSize = Checksum & {size: number} @@ -62,7 +60,7 @@ export function uploadTheme( .then(() => buildDeleteJob(remoteChecksums, themeFileSystem, theme, session, options, uploadResults)) const workPromise = options?.deferPartialWork - ? themeCreationPromise.then(() => {}) + ? themeCreationPromise : deleteJobPromise.then((result) => result.promise) if (options?.backgroundWorkCatch) { @@ -74,71 +72,9 @@ 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 @@ -301,11 +237,9 @@ 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) { - return Promise.resolve([]) + if (missingAssets.length > 0) { + await bulkUploadThemeAssets(theme.id, missingAssets, session) } - - return bulkUploadThemeAssets(theme.id, missingAssets, session) } interface SyncJob {