Merge pull request #6274 from Shopify/08-14-refactor-writeManifestToBundle

Simplify writeManifestToBundle by not re-creating the manifest

Author: isaacroldan

packages/app/src/cli/services/bundle.test.ts

@@ -24,8 +24,10 @@ describe('writeManifestToBundle', () => {
         manifest: async () => manifestContent,
       } as unknown as AppInterface
 
+      const appManifest = await mockApp.manifest(undefined)
+
       // When
-      await writeManifestToBundle(mockApp, tmpDir, undefined)
+      await writeManifestToBundle(appManifest, tmpDir)
 
       // Then
       const manifestPath = joinPath(tmpDir, 'manifest.json')

packages/app/src/cli/services/bundle.ts

@@ -1,21 +1,15 @@
 // import {AppInterface} from '../models/app/app.js'
-import {AppInterface} from '../models/app/app.js'
+import {AppManifest} from '../models/app/app.js'
 import {AssetUrlSchema, DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
 import {MinimalAppIdentifiers} from '../models/organization.js'
-import {Identifiers} from '../models/app/identifiers.js'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver'
 import {formData, fetch} from '@shopify/cli-kit/node/http'
 import {readFileSync} from '@shopify/cli-kit/node/fs'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {writeFile} from 'fs/promises'
 
-export async function writeManifestToBundle(
-  app: AppInterface,
-  bundlePath: string,
-  identifiers: Identifiers | undefined,
-) {
-  const appManifest = await app.manifest(identifiers)
+export async function writeManifestToBundle(appManifest: AppManifest, bundlePath: string) {
   const manifestPath = joinPath(bundlePath, 'manifest.json')
   await writeFile(manifestPath, JSON.stringify(appManifest, null, 2))
 }

packages/app/src/cli/services/deploy.ts

@@ -211,6 +211,7 @@ export async function deploy(options: DeployOptions) {
 
     await bundleAndBuildExtensions({
       app,
+      appManifest,
       bundlePath,
       identifiers,
       skipBuild: options.skipBuild,

packages/app/src/cli/services/deploy/bundle.test.ts

@@ -1,6 +1,6 @@
 import {bundleAndBuildExtensions} from './bundle.js'
 import {testApp, testFunctionExtension, testThemeExtensions, testUIExtension} from '../../models/app/app.test-data.js'
-import {AppInterface} from '../../models/app/app.js'
+import {AppInterface, AppManifest} from '../../models/app/app.js'
 import * as bundle from '../bundle.js'
 import * as functionBuild from '../function/build.js'
 import {describe, expect, test, vi} from 'vitest'
@@ -11,6 +11,7 @@ vi.mock('../function/build.js')
 
 describe('bundleAndBuildExtensions', () => {
   let app: AppInterface
+  let appManifest: AppManifest
 
   test('generates a manifest.json', async () => {
     await file.inTemporaryDirectory(async (tmpDir: string) => {
@@ -36,39 +37,21 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
-      const expectedManifest = {
-        name: 'App',
-        handle: '',
-        modules: [
-          {
-            type: 'web_pixel_extension_external',
-            handle: 'test-ui-extension',
-            uid: 'test-ui-extension-uid',
-            assets: 'test-ui-extension-uid',
-            target: '',
-            config: {},
-          },
-          {
-            type: 'theme_external',
-            handle: 'theme-extension-name',
-            uid: themeExtension.uid,
-            assets: themeExtension.uid,
-            target: '',
-            config: {
-              theme_extension: {
-                files: {},
-              },
-            },
-          },
-        ],
-      }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: false, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: false,
+        isDevDashboardApp: false,
+      })
 
       // Then
       expect(extensionBundleMock).toHaveBeenCalledTimes(2)
-      expect(bundle.writeManifestToBundle).toHaveBeenCalledWith(app, bundleDirectory, identifiers)
+      expect(bundle.writeManifestToBundle).toHaveBeenCalledWith(appManifest, bundleDirectory)
 
       await expect(file.fileExists(bundlePath)).resolves.toBeTruthy()
     })
@@ -96,9 +79,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: false, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: false,
+        isDevDashboardApp: false,
+      })
 
       // Then
       await expect(file.fileExists(bundlePath)).resolves.toBeTruthy()
@@ -129,9 +120,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: true, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: true,
+        isDevDashboardApp: false,
+      })
 
       // Then
       expect(extensionBuildMock).not.toHaveBeenCalled()
@@ -159,9 +158,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: true, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: true,
+        isDevDashboardApp: false,
+      })
 
       // Then
       expect(mockInstallJavy).not.toHaveBeenCalled()
@@ -187,9 +194,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: false, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: false,
+        isDevDashboardApp: false,
+      })
 
       // Then
       expect(mockInstallJavy).toHaveBeenCalledWith(app)
@@ -220,9 +235,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: true, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: true,
+        isDevDashboardApp: false,
+      })
 
       // Then
       expect(extensionBuildMock).not.toHaveBeenCalled()
@@ -280,9 +303,17 @@ describe('bundleAndBuildExtensions', () => {
         extensionIds: {},
         extensionsNonUuidManaged: {},
       }
+      appManifest = await app.manifest(identifiers)
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath, skipBuild: true, isDevDashboardApp: false})
+      await bundleAndBuildExtensions({
+        app,
+        appManifest,
+        identifiers,
+        bundlePath,
+        skipBuild: true,
+        isDevDashboardApp: false,
+      })
 
       // Then - verify none of the build methods were called
       expect(functionBuildMock).not.toHaveBeenCalled()

packages/app/src/cli/services/deploy/bundle.ts

@@ -1,4 +1,4 @@
-import {AppInterface} from '../../models/app/app.js'
+import {AppInterface, AppManifest} from '../../models/app/app.js'
 import {Identifiers} from '../../models/app/identifiers.js'
 import {installJavy} from '../function/build.js'
 import {compressBundle, writeManifestToBundle} from '../bundle.js'
@@ -10,6 +10,7 @@ import {Writable} from 'stream'
 
 interface BundleOptions {
   app: AppInterface
+  appManifest: AppManifest
   bundlePath?: string
   identifiers?: Identifiers
   skipBuild: boolean
@@ -21,7 +22,7 @@ export async function bundleAndBuildExtensions(options: BundleOptions) {
   await rmdir(bundleDirectory, {force: true})
   await mkdir(bundleDirectory)
 
-  await writeManifestToBundle(options.app, bundleDirectory, options.identifiers)
+  await writeManifestToBundle(options.appManifest, bundleDirectory)
 
   // Force the download of the javy binary in advance to avoid later problems,
   // as it might be done multiple times in parallel. https://github.com/Shopify/cli/issues/2877

packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts

@@ -333,7 +333,7 @@ export class DevSession {
     if (this.statusManager.status.isReady) {
       appManifest.modules = appManifest.modules.filter((module) => updatedUids.includes(module.uid))
     } else {
-      await writeManifestToBundle(appEvent.app, this.bundlePath, undefined)
+      await writeManifestToBundle(appManifest, this.bundlePath)
     }
 
     const existingDirs = await readdir(this.bundlePath)