Fixed shopify theme dev to avoid emitting full page reload events w…

…hen files are updated successfully, preventing conflicts with hot-reloading

.changeset/wicked-cats-obey.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fixed `shopify theme dev` to avoid emitting full page reload events when files are updated successfully, preventing conflicts with hot-reloading.

packages/theme/src/cli/utilities/theme-fs.test.ts

@@ -1,4 +1,5 @@
 import {
+  handleSyncUpdate,
   hasRequiredThemeDirectories,
   isJson,
   isTextFile,
@@ -11,10 +12,12 @@ import {getPatternsFromShopifyIgnore, applyIgnoreFilters} from './asset-ignore.j
 import {removeFile, writeFile} from '@shopify/cli-kit/node/fs'
 import {test, describe, expect, vi, beforeEach} from 'vitest'
 import chokidar from 'chokidar'
-import {deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
+import {bulkUploadThemeAssets, deleteThemeAssets, fetchThemeAssets} from '@shopify/cli-kit/node/themes/api'
 import {renderError} from '@shopify/cli-kit/node/ui'
 import {Operation, type Checksum, type ThemeAsset} from '@shopify/cli-kit/node/themes/types'
 import {dirname, joinPath} from '@shopify/cli-kit/node/path'
+import {AdminSession} from '@shopify/cli-kit/node/session'
+import {emitHotReloadEvent} from '@shopify/theme/cli/utilities/theme-environment/hot-reload/server.js'
 import EventEmitter from 'events'
 import {fileURLToPath} from 'node:url'
 
@@ -28,6 +31,7 @@ vi.mock('./asset-ignore.js')
 vi.mock('@shopify/cli-kit/node/themes/api')
 vi.mock('@shopify/cli-kit/node/ui')
 vi.mock('@shopify/cli-kit/node/output')
+vi.mock('@shopify/theme/cli/utilities/theme-environment/hot-reload/server.js')
 
 beforeEach(async () => {
   vi.mocked(getPatternsFromShopifyIgnore).mockResolvedValue([])
@@ -629,6 +633,99 @@ describe('theme-fs', () => {
     })
   })
 
+  describe('handleSyncUpdate', () => {
+    const fileKey = 'assets/test.css'
+    const themeId = '123'
+    const adminSession = {token: 'token'} as AdminSession
+    let unsyncedFileKeys: Set<string>
+    let uploadErrors: Map<string, string[]>
+
+    beforeEach(() => {
+      unsyncedFileKeys = new Set([fileKey])
+      uploadErrors = new Map()
+      vi.mocked(emitHotReloadEvent).mockClear()
+    })
+
+    test('returns false if file is not in unsyncedFileKeys', async () => {
+      // Given
+      unsyncedFileKeys = new Set()
+      const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession)!
+
+      // When
+      const result = await handler('content')
+
+      // Then
+      expect(result).toBe(false)
+      expect(bulkUploadThemeAssets).not.toHaveBeenCalled()
+      expect(emitHotReloadEvent).not.toHaveBeenCalled()
+    })
+
+    test('uploads 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('content')
+
+      // Then
+      expect(result).toBe(true)
+      expect(bulkUploadThemeAssets).toHaveBeenCalledWith(
+        Number(themeId),
+        [{key: fileKey, value: 'content'}],
+        adminSession,
+      )
+      expect(unsyncedFileKeys.has(fileKey)).toBe(false)
+      expect(emitHotReloadEvent).not.toHaveBeenCalled()
+    })
+
+    test('throws error and sets uploadErrors on failed sync', async () => {
+      // Given
+      const errors = ['{{ broken liquid file']
+      vi.mocked(bulkUploadThemeAssets).mockResolvedValue([
+        {
+          key: fileKey,
+          success: false,
+          operation: Operation.Upload,
+          errors: {asset: errors},
+        },
+      ])
+      const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession)!
+
+      // When/Then
+      await expect(handler('content')).rejects.toThrow('{{ broken liquid file')
+      expect(uploadErrors.get(fileKey)).toEqual(errors)
+      expect(unsyncedFileKeys.has(fileKey)).toBe(true)
+      expect(emitHotReloadEvent).toHaveBeenCalledWith({type: 'full', key: fileKey})
+    })
+
+    test('clears uploadErrors if sync succeeds after previous failure', async () => {
+      // Given
+      uploadErrors.set(fileKey, ['Previous error'])
+      vi.mocked(bulkUploadThemeAssets).mockResolvedValue([
+        {
+          key: fileKey,
+          success: true,
+          operation: Operation.Upload,
+        },
+      ])
+      const handler = handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession)!
+
+      // When
+      await handler('content')
+
+      // Then
+      expect(uploadErrors.has(fileKey)).toBe(false)
+      expect(emitHotReloadEvent).toHaveBeenCalledWith({type: 'full', key: fileKey})
+    })
+  })
+
   function fsEntry({key, checksum}: Checksum): [string, ThemeAsset] {
     return [
       key,

packages/theme/src/cli/utilities/theme-fs.ts

@@ -144,26 +144,7 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
     })
 
     const syncPromise = contentPromise
-      .then(async (content) => {
-        if (!unsyncedFileKeys.has(fileKey)) return false
-
-        const [result] = await bulkUploadThemeAssets(Number(themeId), [{key: fileKey, value: content}], adminSession)
-
-        if (result?.success) {
-          uploadErrors.delete(fileKey)
-          emitHotReloadEvent({type: 'full', key: fileKey})
-        } else {
-          const errors = result?.errors?.asset ?? ['Response was not successful.']
-          uploadErrors.set(fileKey, errors)
-          emitHotReloadEvent({type: 'full', key: fileKey})
-          throw new Error(errors.join('\n'))
-        }
-
-        unsyncedFileKeys.delete(fileKey)
-        outputSyncResult('update', fileKey)
-
-        return true
-      })
+      .then(handleSyncUpdate(unsyncedFileKeys, uploadErrors, fileKey, themeId, adminSession))
       .catch(createSyncingCatchError(fileKey, 'upload'))
 
     emitEvent(eventName, {
@@ -267,6 +248,40 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
   }
 }
 
+export function handleSyncUpdate(
+  unsyncedFileKeys: Set<string>,
+  uploadErrors: Map<string, string[]>,
+  fileKey: string,
+  themeId: string,
+  adminSession: AdminSession,
+): ((value: string) => boolean | PromiseLike<boolean>) | null | undefined {
+  return async (content) => {
+    if (!unsyncedFileKeys.has(fileKey)) {
+      return false
+    }
+
+    const [result] = await bulkUploadThemeAssets(Number(themeId), [{key: fileKey, value: content}], adminSession)
+
+    if (!result?.success) {
+      const errors = result?.errors?.asset ?? ['Response was not successful.']
+
+      uploadErrors.set(fileKey, errors)
+      emitHotReloadEvent({type: 'full', key: fileKey})
+
+      throw new Error(errors.join('\n'))
+    }
+
+    if (uploadErrors.delete(fileKey)) {
+      emitHotReloadEvent({type: 'full', key: fileKey})
+    }
+
+    unsyncedFileKeys.delete(fileKey)
+    outputSyncResult('update', fileKey)
+
+    return true
+  }
+}
+
 async function writeThemeFile(root: string, {key, attachment, value}: ThemeAsset) {
   const absolutePath = joinPath(root, key)