Merge pull request #4392 from Shopify/jm/editorsyncprogress

[Themes] Theme Dev - Editor Sync - Fix Hanging Theme Reconciliation Progress Bar and User Inputs

packages/theme/src/cli/services/dev.ts

@@ -95,10 +95,15 @@ export async function dev(options: DevOptions) {
  },
  }
 
+ if (options['theme-editor-sync']) {
+ session.storefrontPassword = await storefrontPasswordPromise
+ }
+
  const {serverStart, renderDevSetupProgress} = setupDevServer(options.theme, ctx)
 
- const storefrontPassword = await storefrontPasswordPromise
- session.storefrontPassword = storefrontPassword
+ if (!options['theme-editor-sync']) {
+ session.storefrontPassword = await storefrontPasswordPromise
+ }
 
  await renderDevSetupProgress()
  await serverStart()

packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.test.ts

@@ -6,7 +6,7 @@ import {fetchChecksums} from '@shopify/cli-kit/node/themes/api'
 import {buildTheme} from '@shopify/cli-kit/node/themes/factories'
 import {ThemeAsset} from '@shopify/cli-kit/node/themes/types'
 import {DEVELOPMENT_THEME_ROLE} from '@shopify/cli-kit/node/themes/utils'
-import {describe, expect, test, vi} from 'vitest'
+import {beforeEach, describe, expect, test, vi} from 'vitest'
 
 vi.mock('@shopify/cli-kit/node/themes/api')
 vi.mock('./theme-reconciliation.js')
@@ -17,17 +17,22 @@ describe('reconcileAndPollThemeEditorChanges', async () => {
  const developmentTheme = buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE})!
  const adminSession = {token: '', storeFqdn: ''}
 
+ beforeEach(() => {
+ vi.mocked(reconcileJsonFiles).mockResolvedValue({
+ workPromise: Promise.resolve(),
+ })
+ })
+
  test('should call pollThemeEditorChanges with updated checksums if the remote theme was been updated during reconciliation', async () => {
  // Given
  const files = new Map<string, ThemeAsset>([])
  const defaultThemeFileSystem = fakeThemeFileSystem('tmp', files)
  const initialRemoteChecksums = [{checksum: '1', key: 'templates/asset.json'}]
 
- vi.mocked(reconcileJsonFiles).mockResolvedValue(undefined)
  vi.mocked(fetchChecksums).mockResolvedValue([{checksum: '2', key: 'templates/asset.json'}])
 
  // When
- await reconcileAndPollThemeEditorChanges(
+ const {workPromise, updatedRemoteChecksumsPromise} = await reconcileAndPollThemeEditorChanges(
  developmentTheme,
  adminSession,
  initialRemoteChecksums,
@@ -38,6 +43,8 @@ describe('reconcileAndPollThemeEditorChanges', async () => {
  only: [],
  },
  )
+ await workPromise
+ await updatedRemoteChecksumsPromise
 
  // Then
  expect(pollThemeEditorChanges).toHaveBeenCalledWith(
@@ -53,34 +60,6 @@ describe('reconcileAndPollThemeEditorChanges', async () => {
  )
  })
 
- test('should not call reconcileJsonFiles when remote theme contains no files', async () => {
- // Given
- const files = new Map<string, ThemeAsset>([])
- const defaultThemeFileSystem = fakeThemeFileSystem('tmp', files)
- const emptyRemoteChecksums: [] = []
- const newFileSystem = fakeThemeFileSystem('tmp', new Map<string, ThemeAsset>([]))
-
- vi.mocked(fetchChecksums).mockResolvedValue([])
- const readySpy = vi.spyOn(defaultThemeFileSystem, 'ready').mockResolvedValue()
-
- // When
- await reconcileAndPollThemeEditorChanges(
- developmentTheme,
- adminSession,
- emptyRemoteChecksums,
- defaultThemeFileSystem,
- {
- noDelete: false,
- ignore: [],
- only: [],
- },
- )
-
- // Then
- expect(reconcileJsonFiles).not.toHaveBeenCalled()
- expect(pollThemeEditorChanges).toHaveBeenCalled()
- })
-
  test('should wait for the local theme file system to be ready before reconciling', async () => {
  // Given
  const files = new Map<string, ThemeAsset>([])

packages/theme/src/cli/utilities/theme-environment/remote-theme-watcher.ts

@@ -1,9 +1,9 @@
 import {pollThemeEditorChanges} from './theme-polling.js'
 import {reconcileJsonFiles} from './theme-reconciliation.js'
+import {outputDebug} from '@shopify/cli-kit/node/output'
 import {AdminSession} from '@shopify/cli-kit/node/session'
 import {Checksum, Theme, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types'
 import {fetchChecksums} from '@shopify/cli-kit/node/themes/api'
-import {outputDebug} from '@shopify/cli-kit/node/output'
 
 export const LOCAL_STRATEGY = 'local'
 export const REMOTE_STRATEGY = 'remote'
@@ -22,16 +22,20 @@ export async function reconcileAndPollThemeEditorChanges(
  ignore: string[]
  only: string[]
  },
-) {
+): Promise<{
+ updatedRemoteChecksumsPromise: Promise<Checksum[]>
+ workPromise: Promise<void>
+}> {
  outputDebug('Initiating theme asset reconciliation process')
  await localThemeFileSystem.ready()
 
- if (remoteChecksums.length !== 0) {
- await reconcileJsonFiles(targetTheme, session, remoteChecksums, localThemeFileSystem, options)
- }
+ const {workPromise} = await reconcileJsonFiles(targetTheme, session, remoteChecksums, localThemeFileSystem, options)
 
- const updatedRemoteChecksums = await fetchChecksums(targetTheme.id, session)
- pollThemeEditorChanges(targetTheme, session, updatedRemoteChecksums, localThemeFileSystem, options)
+ const updatedRemoteChecksumsPromise = workPromise.then(async () => {
+ const updatedRemoteChecksums = await fetchChecksums(targetTheme.id, session)
+ pollThemeEditorChanges(targetTheme, session, updatedRemoteChecksums, localThemeFileSystem, options)
+ return updatedRemoteChecksums
+ })
 
- return updatedRemoteChecksums
+ return {updatedRemoteChecksumsPromise, workPromise}
 }

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

@@ -104,6 +104,10 @@ describe('setupDevServer', () => {
  ...filters,
  },
  }
+ vi.mocked(reconcileAndPollThemeEditorChanges).mockResolvedValue({
+ updatedRemoteChecksumsPromise: Promise.resolve([]),
+ workPromise: Promise.resolve(),
+ })
 
  // When
  await setupDevServer(developmentTheme, context).workPromise

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

@@ -1,9 +1,10 @@
-import {reconcileAndPollThemeEditorChanges} from './remote-theme-watcher.js'
 import {getHotReloadHandler, setupInMemoryTemplateWatcher} from './hot-reload/server.js'
 import {getHtmlHandler} from './html.js'
 import {getAssetsHandler} from './local-assets.js'
 import {getProxyHandler} from './proxy.js'
+import {reconcileAndPollThemeEditorChanges} from './remote-theme-watcher.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'
@@ -28,32 +29,61 @@ 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,
- ignore: ctx.options.ignore,
- only: ctx.options.only,
- })
- : remoteChecksums,
+ handleThemeEditorSync(theme, ctx, remoteChecksums),
  )
 
- const uploadPromise = reconcilePromise.then(async (remoteChecksums: Checksum[]) =>
- uploadTheme(theme, ctx.session, remoteChecksums, ctx.localThemeFileSystem, {
+ const uploadPromise = reconcilePromise.then(async ({updatedRemoteChecksumsPromise}) => {
+ const updatedRemoteChecksums = await updatedRemoteChecksumsPromise
+ return uploadTheme(theme, ctx.session, updatedRemoteChecksums, ctx.localThemeFileSystem, {
  nodelete: ctx.options.noDelete,
  deferPartialWork: true,
- }),
- )
+ })
+ })
 
  return {
  workPromise: uploadPromise.then((result) => result.workPromise),
  renderProgress: async () => {
+ if (ctx.options.themeEditorSync) {
+ const {workPromise} = await reconcilePromise
+ await renderTasksToStdErr([
+ {
+ title: 'Performing file synchronization. This may take a while...',
+ task: async () => {
+ await workPromise
+ },
+ },
+ ])
+ }
+
  const {renderThemeSyncProgress} = await uploadPromise
 
  await renderThemeSyncProgress()
  },
  }
 }
 
+function handleThemeEditorSync(
+ theme: Theme,
+ ctx: DevServerContext,
+ remoteChecksums: Checksum[],
+): Promise<{
+ updatedRemoteChecksumsPromise: Promise<Checksum[]>
+ workPromise: 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({
+ updatedRemoteChecksumsPromise: Promise.resolve(remoteChecksums),
+ workPromise: Promise.resolve(),
+ })
+ }
+}
+
 interface DevelopmentServerInstance {
  close: () => Promise<void>
 }