Merge pull request #3919 from Shopify/jmeng/optimize_poll

[Themes] Optimize theme polling to skip re-downloading recently uploaded files

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

@@ -1,4 +1,4 @@
-import * as themePolling from './theme-polling.js'
+import {pollRemoteJsonChanges} from './theme-polling.js'
 import {readThemeFilesFromDisk} from '../theme-fs.js'
 import {fakeThemeFileSystem} from '../theme-fs/theme-fs-mock-factory.js'
 import {AbortError} from '@shopify/cli-kit/node/error'
@@ -12,7 +12,6 @@ vi.mock('@shopify/cli-kit/node/themes/api')
 vi.mock('../theme-fs.js')
 
 describe('pollRemoteJsonChanges', async () => {
-  const {pollRemoteJsonChanges} = themePolling
   const developmentTheme = buildTheme({id: 1, name: 'Theme', role: DEVELOPMENT_THEME_ROLE})!
   const adminSession = {token: '', storeFqdn: ''}
   const files = new Map<string, ThemeAsset>([])
@@ -54,6 +53,27 @@ describe('pollRemoteJsonChanges', async () => {
     })
   })
 
+  test('does not download newly added files from remote theme when file with equivalent checksum is already presenty locally', async () => {
+    // Given
+    const remoteChecksums: Checksum[] = []
+    const updatedRemoteChecksums = [{checksum: '1', key: 'templates/asset.json'}]
+    vi.mocked(fetchChecksums).mockResolvedValue(updatedRemoteChecksums)
+    const themeFileSystem = fakeThemeFileSystem(
+      'tmp',
+      new Map([['templates/asset.json', {checksum: '1', key: 'templates/asset.json', value: 'content'}]]),
+    )
+
+    // When
+    await pollRemoteJsonChanges(developmentTheme, adminSession, remoteChecksums, themeFileSystem)
+
+    // Then
+    expect(themeFileSystem.files.get('templates/asset.json')).toEqual({
+      checksum: '1',
+      key: 'templates/asset.json',
+      value: 'content',
+    })
+  })
+
   test('deletes local file from remote theme when there is a change on remote', async () => {
     // Given
     const remoteChecksums = [{checksum: '1', key: 'templates/asset.json'}]

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

@@ -1,4 +1,4 @@
-import {Checksum, Theme, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types'
+import {Checksum, Theme, ThemeAsset, ThemeFileSystem} from '@shopify/cli-kit/node/themes/types'
 import {fetchChecksums, fetchThemeAsset} from '@shopify/cli-kit/node/themes/api'
 import {outputDebug} from '@shopify/cli-kit/node/output'
 import {AdminSession} from '@shopify/cli-kit/node/session'
@@ -51,12 +51,17 @@ export async function pollRemoteJsonChanges(
     return latestChecksumsMap.get(previousChecksum.key) === undefined
   })
 
-  await abortIfMultipleSourcesChange(localFileSystem, assetsChangedOnRemote)
+  const previousFileValues = new Map(localFileSystem.files)
+  await Promise.all(assetsChangedOnRemote.map((file) => localFileSystem.read(file.key)))
+  await abortIfMultipleSourcesChange(previousFileValues, localFileSystem, assetsChangedOnRemote)
 
   await Promise.all(
     assetsChangedOnRemote
       .filter((file) => file.key.endsWith('.json'))
       .map(async (file) => {
+        if (localFileSystem.files.get(file.key)?.checksum === file.checksum) {
+          return
+        }
         const asset = await fetchThemeAsset(targetTheme.id, file.key, currentSession)
         if (asset) {
           return localFileSystem.write(asset).then(() => {
@@ -78,11 +83,13 @@ export async function pollRemoteJsonChanges(
   return latestChecksums
 }
 
-async function abortIfMultipleSourcesChange(localFileSystem: ThemeFileSystem, assetsChangedOnRemote: Checksum[]) {
+async function abortIfMultipleSourcesChange(
+  previousFileValues: Map<string, ThemeAsset>,
+  localFileSystem: ThemeFileSystem,
+  assetsChangedOnRemote: Checksum[],
+) {
   for (const asset of assetsChangedOnRemote) {
-    const previousChecksum = localFileSystem.files.get(asset.key)?.checksum
-    // eslint-disable-next-line no-await-in-loop
-    await localFileSystem.read(asset.key)
+    const previousChecksum = previousFileValues.get(asset.key)?.checksum
     const newChecksum = localFileSystem.files.get(asset.key)?.checksum
     if (previousChecksum !== newChecksum) {
       throw new PollingError(