From 57bb933980448a962d8ec287f3d6c3157bf85528 Mon Sep 17 00:00:00 2001
From: Josh Faigan <josh.faigan@shopify.com>
Date: Wed, 16 Oct 2024 13:25:36 -0400
Subject: [PATCH] Fix bug when using --nodelete flag on dev command

This fixes an issue where the --nodelete flag was not being respected when
running the dev command. This was due to the fact that the flag was not
being passed to the theme-fs utility function.
---
 .changeset/strong-guests-worry.md             |  5 +++
 .../cli-kit/src/public/node/themes/types.ts   |  1 +
 packages/theme/src/cli/services/dev.ts        |  1 +
 .../theme/src/cli/utilities/theme-fs.test.ts  | 33 +++++++++++++++++++
 packages/theme/src/cli/utilities/theme-fs.ts  | 24 +++++++-------
 5 files changed, 53 insertions(+), 11 deletions(-)
 create mode 100644 .changeset/strong-guests-worry.md

diff --git a/.changeset/strong-guests-worry.md b/.changeset/strong-guests-worry.md
new file mode 100644
index 0000000000..3f431cddff
--- /dev/null
+++ b/.changeset/strong-guests-worry.md
@@ -0,0 +1,5 @@
+---
+'@shopify/cli': patch
+---
+
+Fix theme dev command deleting remote files even if using --nodelete flag
diff --git a/packages/cli-kit/src/public/node/themes/types.ts b/packages/cli-kit/src/public/node/themes/types.ts
index 446f9b0ee3..1fdf1e6d2e 100644
--- a/packages/cli-kit/src/public/node/themes/types.ts
+++ b/packages/cli-kit/src/public/node/themes/types.ts
@@ -26,6 +26,7 @@ export type ThemeFSEventPayload<T extends ThemeFSEventName = 'add'> = (ThemeFSEv
 export interface ThemeFileSystemOptions {
   filters?: {ignore?: string[]; only?: string[]}
   notify?: string
+  noDelete?: boolean
 }
 
 /**
diff --git a/packages/theme/src/cli/services/dev.ts b/packages/theme/src/cli/services/dev.ts
index cf4adcd873..acfd9333be 100644
--- a/packages/theme/src/cli/services/dev.ts
+++ b/packages/theme/src/cli/services/dev.ts
@@ -49,6 +49,7 @@ export async function dev(options: DevOptions) {
   const localThemeFileSystem = mountThemeFileSystem(options.directory, {
     filters: options,
     notify: options.notify,
+    noDelete: options.noDelete,
   })
 
   const host = options.host ?? DEFAULT_HOST
diff --git a/packages/theme/src/cli/utilities/theme-fs.test.ts b/packages/theme/src/cli/utilities/theme-fs.test.ts
index c729becbd2..fa81b0688a 100644
--- a/packages/theme/src/cli/utilities/theme-fs.test.ts
+++ b/packages/theme/src/cli/utilities/theme-fs.test.ts
@@ -529,6 +529,39 @@ describe('theme-fs', () => {
       expect(deleteThemeAsset).toHaveBeenCalledWith(Number(themeId), 'assets/base.css', adminSession)
     })
 
+    test('does not delete file from remote when options.noDelete is true', async () => {
+      // Given
+      vi.mocked(fetchThemeAsset).mockResolvedValue({
+        key: 'assets/base.css',
+        checksum: '1',
+        value: 'content',
+        attachment: '',
+        stats: {size: 100, mtime: 100},
+      })
+      vi.mocked(deleteThemeAsset).mockResolvedValue(true)
+
+      // When
+      const themeFileSystem = mountThemeFileSystem(root, {noDelete: true})
+      await themeFileSystem.ready()
+
+      const deleteOperationPromise = new Promise<void>((resolve) => {
+        themeFileSystem.addEventListener('unlink', () => {
+          setImmediate(resolve)
+        })
+      })
+
+      await themeFileSystem.startWatcher(themeId, adminSession)
+
+      // Explicitly emit the 'unlink' event
+      const watcher = chokidar.watch('') as EventEmitter
+      watcher.emit('unlink', `${root}/assets/base.css`)
+
+      await deleteOperationPromise
+
+      // Then
+      expect(deleteThemeAsset).not.toHaveBeenCalled()
+    })
+
     test('renders a warning to debug if the file deletion fails', async () => {
       // Given
       vi.mocked(fetchThemeAsset).mockResolvedValue({
diff --git a/packages/theme/src/cli/utilities/theme-fs.ts b/packages/theme/src/cli/utilities/theme-fs.ts
index 72d607c394..1b6b211f0e 100644
--- a/packages/theme/src/cli/utilities/theme-fs.ts
+++ b/packages/theme/src/cli/utilities/theme-fs.ts
@@ -189,17 +189,19 @@ export function mountThemeFileSystem(root: string, options?: ThemeFileSystemOpti
     files.delete(fileKey)
     unsyncedFileKeys.add(fileKey)
 
-    const syncPromise = deleteThemeAsset(Number(themeId), fileKey, adminSession)
-      .then(async (success) => {
-        if (!success) throw new Error(`Failed to delete file "${fileKey}" from remote theme.`)
-        unsyncedFileKeys.delete(fileKey)
-        outputSyncResult('delete', fileKey)
-        return true
-      })
-      .catch((error) => {
-        createSyncingCatchError(fileKey, 'delete')(error)
-        return false
-      })
+    const syncPromise = options?.noDelete
+      ? Promise.resolve()
+      : deleteThemeAsset(Number(themeId), fileKey, adminSession)
+          .then(async (success) => {
+            if (!success) throw new Error(`Failed to delete file "${fileKey}" from remote theme.`)
+            unsyncedFileKeys.delete(fileKey)
+            outputSyncResult('delete', fileKey)
+            return true
+          })
+          .catch((error) => {
+            createSyncingCatchError(fileKey, 'delete')(error)
+            return false
+          })
 
     emitEvent('unlink', {
       fileKey,