Notify users when Liquid files are rewritten remotely

.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

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'}},
                     ],
                   },

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

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',
+        },
       },
     ])
   })

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 ?? '',
+      },
     })
   })
 

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(),
   ])
 }
 

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

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)

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)
 }
 

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
 

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',
     })
   })
 

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),
   }
 }
 

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)