Merge pull request #6589 from Shopify/revert-6524-10-16-surface_rewritten_liquid_files_to_dev_and_push_commands

aswamy · web-flow · commit c356b4e29b90 · 2025-11-10T15:07:21.000Z
Revert "Surface rewritten liquid files to dev and push commands"
diff --git a/.changeset/honest-corners-relate.md 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -101,15 +101,14 @@ 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
   }
 
   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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<string>
     let uploadErrors: Map<string, string[]>
-    const write = vi.fn()
 
     beforeEach(() => {
       unsyncedFileKeys = new Set([fileKey])
@@ -717,79 +710,45 @@ 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)
       expect(bulkUploadThemeAssets).not.toHaveBeenCalled()
       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
diff --git a/packages/theme/src/cli/utilities/theme-uploader.test.ts b/packages/theme/src/cli/utilities/theme-uploader.test.ts
diff --git a/packages/theme/src/cli/utilities/theme-uploader.ts b/packages/theme/src/cli/utilities/theme-uploader.ts

Original file line number	Diff line number	Diff line change
`@@ -2,7 +2,6 @@ mutation themeFilesUpsert($files: [OnlineStoreThemeFilesUpsertFileInput!]!, $the`
`2`	`2`	`themeFilesUpsert(files: $files, themeId: $themeId) {`
`3`	`3`	`upsertedThemeFiles {`
`4`	`4`	`filename`
`5`		`- checksumMd5`
`6`	`5`	`}`
`7`	`6`	`userErrors {`
`8`	`7`	`filename`
Original file line number	Diff line number	Diff line change
`@@ -101,15 +101,14 @@ export async function dev(options: DevOptions) {`
`101`	`101`	`session.storefrontPassword = await storefrontPasswordPromise`
`102`	`102`	`}`
`103`	`103`
`104`		`- const {serverStart, renderDevSetupProgress, syncRewrittenFilesPromise} = setupDevServer(options.theme, ctx)`
	`104`	`+ const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx)`
`105`	`105`
`106`	`106`	`if (!options['theme-editor-sync']) {`
`107`	`107`	`session.storefrontPassword = await storefrontPasswordPromise`
`108`	`108`	`}`
`109`	`109`
`110`	`110`	`await renderDevSetupProgress()`
`111`	`111`	`await serverStart()`
`112`		`- await syncRewrittenFilesPromise()`
`113`	`112`
`114`	`113`	`renderLinks(urls)`
`115`	`114`	`if (options.open) {`