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

aswamy · web-flow · commit 1b47f9a6dcaf · 2025-11-05T08:08:57.000Z
Surface rewritten liquid files to dev and push commands
diff --git a/.changeset/sour-boats-grab.md 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
@@ -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
@@ -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
@@ -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
@@ -315,6 +315,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
@@ -101,14 +101,15 @@ export async function dev(options: DevOptions) {
     session.storefrontPassword = await storefrontPasswordPromise
   }
 
-  const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx)
+  const {serverStart, renderDevSetupProgress, syncRewrittenFilesPromise} = 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,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
@@ -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/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
@@ -34,7 +34,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(),
+    }
   })
 })
 
@@ -126,6 +131,7 @@ describe('setupDevServer', () => {
       nodelete: true,
       deferPartialWork: true,
       backgroundWorkCatch: expect.any(Function),
+      handleRewrittenFiles: 'fix',
     })
   })
 
@@ -176,6 +182,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
@@ -25,6 +25,7 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) {
     serverStart: server.start,
     dispatchEvent: server.dispatch,
     renderDevSetupProgress: envSetup.renderProgress,
+    syncRewrittenFilesPromise: () => envSetup.syncRewrittenFilesPromise,
   }
 }
 
@@ -44,6 +45,7 @@ function ensureThemeEnvironmentSetup(theme: Theme, ctx: DevServerContext) {
         nodelete: ctx.options.noDelete,
         deferPartialWork: true,
         backgroundWorkCatch: abort,
+        handleRewrittenFiles: 'fix',
       })
     })
     .catch(abort)
@@ -67,6 +69,7 @@ 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,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<string>
     let uploadErrors: Map<string, string[]>
+    const write = vi.fn()
 
     beforeEach(() => {
       unsyncedFileKeys = new Set([fileKey])
@@ -710,45 +717,79 @@ 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)
       expect(bulkUploadThemeAssets).not.toHaveBeenCalled()
       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
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,6 +2,7 @@ mutation themeFilesUpsert($files: [OnlineStoreThemeFilesUpsertFileInput!]!, $the`
`2`	`2`	`themeFilesUpsert(files: $files, themeId: $themeId) {`
`3`	`3`	`upsertedThemeFiles {`
`4`	`4`	`filename`
	`5`	`+ checksumMd5`
`5`	`6`	`}`
`6`	`7`	`userErrors {`
`7`	`8`	`filename`
Original file line number	Diff line number	Diff line change
`@@ -101,14 +101,15 @@ export async function dev(options: DevOptions) {`
`101`	`101`	`session.storefrontPassword = await storefrontPasswordPromise`
`102`	`102`	`}`
`103`	`103`
`104`		`- const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx)`
	`104`	`+ const {serverStart, renderDevSetupProgress, syncRewrittenFilesPromise} = 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()`
`112`	`113`
`113`	`114`	`renderLinks(urls)`
`114`	`115`	`if (options.open) {`