Fix Hanging Theme Reconciliation Progress Bar

Shopify · Aug 30, 2024 · ca07b64 · ca07b64
1 parent d7dd0a8
commit ca07b64
Show file tree

Hide file tree

Showing 6 changed files with 168 additions and 39 deletions.
diff --git a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts
@@ -23,7 +23,10 @@ describe('reconcileAndPollThemeEditorChanges', async () => {
     const defaultThemeFileSystem = fakeThemeFileSystem('tmp', files)
     const initialRemoteChecksums = [{checksum: '1', key: 'templates/asset.json'}]
 
-    vi.mocked(reconcileJsonFiles).mockResolvedValue(undefined)
+    vi.mocked(reconcileJsonFiles).mockResolvedValue({
+      reconciliationFinishedPromise: Promise.resolve(),
+      readyForReconciliationPromise: Promise.resolve(),
+    })
     vi.mocked(fetchChecksums).mockResolvedValue([{checksum: '2', key: 'templates/asset.json'}])
 
     // When

diff --git a/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts b/packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts
@@ -22,16 +22,24 @@ export async function reconcileAndPollThemeEditorChanges(
     ignore: string[]
     only: string[]
   },
-) {
+): Promise<{
+  updatedRemoteChecksums: Checksum[]
+  reconciliationFinishedPromise: Promise<void>
+  readyForReconciliationPromise: Promise<void>
+}> {
   outputDebug('Initiating theme asset reconciliation process')
   await localThemeFileSystem.ready()
 
-  if (remoteChecksums.length !== 0) {
-    await reconcileJsonFiles(targetTheme, session, remoteChecksums, localThemeFileSystem, options)
-  }
+  const {reconciliationFinishedPromise, readyForReconciliationPromise} = await reconcileJsonFiles(
+    targetTheme,
+    session,
+    remoteChecksums,
+    localThemeFileSystem,
+    options,
+  )
 
   const updatedRemoteChecksums = await fetchChecksums(targetTheme.id, session)
   pollThemeEditorChanges(targetTheme, session, updatedRemoteChecksums, localThemeFileSystem, options)
 
-  return updatedRemoteChecksums
+  return {updatedRemoteChecksums, reconciliationFinishedPromise, readyForReconciliationPromise}
 }
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
@@ -104,6 +104,11 @@ describe('setupDevServer', () => {
         ...filters,
       },
     }
+    vi.mocked(reconcileAndPollThemeEditorChanges).mockResolvedValue({
+      updatedRemoteChecksums: [],
+      readyForReconciliationPromise: Promise.resolve(),
+      reconciliationFinishedPromise: Promise.resolve(),
+    })
 
     // When
     await setupDevServer(developmentTheme, context).workPromise

diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.ts
@@ -4,6 +4,7 @@ import {getHtmlHandler} from './html.js'
 import {getAssetsHandler} from './local-assets.js'
 import {getProxyHandler} from './proxy.js'
 import {uploadTheme} from '../theme-uploader.js'
+import {renderTasksToStdErr} from '../theme-ui.js'
 import {createApp, defineEventHandler, defineLazyEventHandler, toNodeListener} from 'h3'
 import {fetchChecksums} from '@shopify/cli-kit/node/themes/api'
 import {createServer} from 'node:http'
@@ -27,33 +28,73 @@ export function setupDevServer(theme: Theme, ctx: DevServerContext) {
 function ensureThemeEnvironmentSetup(theme: Theme, ctx: DevServerContext) {
   const remoteChecksumsPromise = fetchChecksums(theme.id, ctx.session)
 
-  const reconcilePromise = remoteChecksumsPromise.then((remoteChecksums) =>
-    ctx.options.themeEditorSync
-      ? reconcileAndPollThemeEditorChanges(theme, ctx.session, remoteChecksums, ctx.localThemeFileSystem, {
-          noDelete: ctx.options.noDelete,
+  const editorSyncPromise = remoteChecksumsPromise.then((remoteChecksums) =>
+    handleThemeEditorSync(theme, ctx, remoteChecksums),
+  )
+
+  const themeSetupPromise = editorSyncPromise.then(
+    ({updatedRemoteChecksums, reconciliationFinishedPromise, readyForReconciliationPromise}) => {
+      const uploadPromise = reconciliationFinishedPromise.then(() =>
+        uploadTheme(theme, ctx.session, updatedRemoteChecksums, ctx.localThemeFileSystem, {
+          nodelete: ctx.options.noDelete,
           ignore: ctx.options.ignore,
           only: ctx.options.only,
-        })
-      : remoteChecksums,
-  )
+          deferPartialWork: true,
+        }),
+      )
 
-  const uploadPromise = reconcilePromise.then(async (remoteChecksums: Checksum[]) =>
-    uploadTheme(theme, ctx.session, remoteChecksums, ctx.localThemeFileSystem, {
-      nodelete: ctx.options.noDelete,
-      deferPartialWork: true,
-    }),
+      return {uploadPromise, reconciliationFinishedPromise, readyForReconciliationPromise}
+    },
   )
 
   return {
-    workPromise: uploadPromise.then((result) => result.workPromise),
+    workPromise: themeSetupPromise.then(async ({uploadPromise}) => (await uploadPromise).workPromise),
     renderProgress: async () => {
-      const {renderThemeSyncProgress} = await uploadPromise
+      if (ctx.options.themeEditorSync) {
+        await themeSetupPromise.then(({readyForReconciliationPromise}) => readyForReconciliationPromise)
+        await renderTasksToStdErr([
+          {
+            title: 'Performing file synchronization. This may take a while...',
+            task: async () => {
+              await themeSetupPromise.then(async ({reconciliationFinishedPromise}) => {
+                await reconciliationFinishedPromise
+              })
+            },
+          },
+        ])
+      }
+
+      const {renderThemeSyncProgress} = await themeSetupPromise.then(({uploadPromise}) => uploadPromise)
 
       await renderThemeSyncProgress()
     },
   }
 }
 
+function handleThemeEditorSync(
+  theme: Theme,
+  ctx: DevServerContext,
+  remoteChecksums: Checksum[],
+): Promise<{
+  updatedRemoteChecksums: Checksum[]
+  reconciliationFinishedPromise: Promise<void>
+  readyForReconciliationPromise: Promise<void>
+}> {
+  if (ctx.options.themeEditorSync) {
+    return reconcileAndPollThemeEditorChanges(theme, ctx.session, remoteChecksums, ctx.localThemeFileSystem, {
+      noDelete: ctx.options.noDelete,
+      ignore: ctx.options.ignore,
+      only: ctx.options.only,
+    })
+  } else {
+    return Promise.resolve({
+      updatedRemoteChecksums: remoteChecksums,
+      reconciliationFinishedPromise: Promise.resolve(),
+      readyForReconciliationPromise: Promise.resolve(),
+    })
+  }
+}
+
 interface DevelopmentServerInstance {
   close: () => Promise<void>
 }

diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-reconciliation.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-reconciliation.test.ts
@@ -33,7 +33,13 @@ describe('reconcileThemeFiles', () => {
       ]
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, defaultThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        defaultThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(fetchThemeAsset).toHaveBeenCalledTimes(1)
@@ -51,10 +57,16 @@ describe('reconcileThemeFiles', () => {
       ]
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, defaultThemeFileSystem, {
-        ...defaultOptions,
-        only: ['templates/*'],
-      })
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        defaultThemeFileSystem,
+        {
+          ...defaultOptions,
+          only: ['templates/*'],
+        },
+      )
 
       // Then
       expect(fetchThemeAsset).toHaveBeenCalledTimes(1)
@@ -93,7 +105,13 @@ describe('reconcileThemeFiles', () => {
 
       // When
       expect(defaultThemeFileSystem.files.get('templates/asset.json')).toBeUndefined()
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, defaultThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        defaultThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(fetchThemeAsset).toHaveBeenCalledWith(developmentTheme.id, assetToBeDownloaded.key, adminSession)
@@ -107,7 +125,13 @@ describe('reconcileThemeFiles', () => {
       const remoteChecksums = [assetToBeDeleted]
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, defaultThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        defaultThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(deleteThemeAsset).toHaveBeenCalledWith(developmentTheme.id, assetToBeDeleted.key, adminSession)
@@ -124,7 +148,13 @@ describe('reconcileThemeFiles', () => {
       const spy = vi.spyOn(localThemeFileSystem, 'delete')
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, localThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        localThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(spy).toHaveBeenCalledWith('templates/asset.json')
@@ -138,7 +168,13 @@ describe('reconcileThemeFiles', () => {
       const spy = vi.spyOn(localThemeFileSystem, 'delete')
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, localThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        localThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(spy).not.toHaveBeenCalled()
@@ -151,10 +187,16 @@ describe('reconcileThemeFiles', () => {
       const spy = vi.spyOn(localThemeFileSystem, 'delete')
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, localThemeFileSystem, {
-        ...defaultOptions,
-        noDelete: true,
-      })
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        localThemeFileSystem,
+        {
+          ...defaultOptions,
+          noDelete: true,
+        },
+      )
 
       // Then
       expect(renderSelectPrompt).not.toHaveBeenCalled()
@@ -171,7 +213,13 @@ describe('reconcileThemeFiles', () => {
       const remoteChecksums = [{checksum: '2', key: 'templates/asset.json'}]
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, localThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        localThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(fetchThemeAsset).toHaveBeenCalled()
@@ -185,10 +233,21 @@ describe('reconcileThemeFiles', () => {
       const remoteChecksums = [{checksum: '2', key: 'templates/asset.json'}]
 
       // When
-      await reconcileJsonFiles(developmentTheme, adminSession, remoteChecksums, localThemeFileSystem, defaultOptions)
+      await reconcileAndWaitForReconciliationFinish(
+        developmentTheme,
+        adminSession,
+        remoteChecksums,
+        localThemeFileSystem,
+        defaultOptions,
+      )
 
       // Then
       expect(fetchThemeAsset).not.toHaveBeenCalled()
     })
   })
 })
+
+async function reconcileAndWaitForReconciliationFinish(...args: Parameters<typeof reconcileJsonFiles>) {
+  const {reconciliationFinishedPromise} = await reconcileJsonFiles(...args)
+  return reconciliationFinishedPromise
+}
diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-reconciliation.ts b/packages/theme/src/cli/utilities/theme-environment/theme-reconciliation.ts
@@ -25,7 +25,10 @@ export async function reconcileJsonFiles(
   remoteChecksums: Checksum[],
   localThemeFileSystem: ThemeFileSystem,
   options: ReconciliationOptions,
-) {
+): Promise<{
+  readyForReconciliationPromise: Promise<void>
+  reconciliationFinishedPromise: Promise<void>
+}> {
   outputDebug('Initiating theme asset reconciliation process')
 
   const {filesOnlyPresentLocally, filesOnlyPresentOnRemote, filesWithConflictingChecksums} = identifyFilesToReconcile(
@@ -39,10 +42,13 @@ export async function reconcileJsonFiles(
     filesWithConflictingChecksums.length === 0
   ) {
     outputDebug('Local and remote checksums match - no need to reconcile theme assets')
-    return localThemeFileSystem
+    return {
+      readyForReconciliationPromise: Promise.resolve(),
+      reconciliationFinishedPromise: Promise.resolve(),
+    }
   }
 
-  const partitionedFiles = await partitionFilesByReconciliationStrategy(
+  const readyForReconciliationPromise = partitionFilesByReconciliationStrategy(
     {
       filesOnlyPresentLocally,
       filesOnlyPresentOnRemote,
@@ -51,7 +57,14 @@ export async function reconcileJsonFiles(
     options,
   )
 
-  await performFileReconciliation(targetTheme, session, localThemeFileSystem, partitionedFiles)
+  const reconciliationFinishedPromise = readyForReconciliationPromise.then((partitionedFiles) =>
+    performFileReconciliation(targetTheme, session, localThemeFileSystem, partitionedFiles),
+  )
+
+  return {
+    readyForReconciliationPromise: readyForReconciliationPromise.then(() => {}),
+    reconciliationFinishedPromise,
+  }
 }
 
 function identifyFilesToReconcile(