Clean up ExtensionSpecification spreads and mutations

packages/app/src/cli/models/extensions/specification.ts

@@ -1,7 +1,7 @@
 import {ZodSchemaType, BaseConfigType, BaseSchema} from './schemas.js'
 import {ExtensionInstance} from './extension-instance.js'
 import {blocks} from '../../constants.js'
-
+import {FlattenedRemoteSpecification} from '../../api/graphql/extension_specifications.js'
 import {Flag} from '../../utilities/developer-platform-client.js'
 import {AppConfiguration} from '../app/app.js'
 import {loadLocalesConfig} from '../../utilities/extensions/locales-configuration.js'
@@ -166,6 +166,29 @@ export class ExtensionSpecification<TConfiguration extends BaseConfigType = Base
       }
     }
   }
+
+  loadedRemoteSpecs?: true
+
+  applyRemoteSpecification(remoteSpec: FlattenedRemoteSpecification): RemoteAwareExtensionSpecification<TConfiguration> {
+    this.registrationLimit = remoteSpec.registrationLimit
+    this.externalIdentifier = remoteSpec.externalIdentifier
+    this.externalName = remoteSpec.externalName
+    this.experience = remoteSpec.experience as ExtensionExperience
+    if (remoteSpec.surface) {
+      this.surface = remoteSpec.surface
+    }
+    // WORKAROUND: The value from the API is wrong for this extension
+    if (remoteSpec.identifier === 'checkout_post_purchase') {
+      this.surface = 'post_purchase'
+    }
+    this.loadedRemoteSpecs = true
+    return this as RemoteAwareExtensionSpecification<TConfiguration>
+  }
+
+  markAsRemoteLoaded(): RemoteAwareExtensionSpecification<TConfiguration> {
+    this.loadedRemoteSpecs = true
+    return this as RemoteAwareExtensionSpecification<TConfiguration>
+  }
 }
 
 /**
@@ -241,12 +264,13 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
 }
 
 export function createContractBasedModuleSpecification<TConfiguration extends BaseConfigType = BaseConfigType>(
-  spec: Pick<CreateExtensionSpecType<TConfiguration>, 'identifier' | 'appModuleFeatures' | 'buildConfig'>,
+  spec: Pick<CreateExtensionSpecType<TConfiguration>, 'identifier' | 'appModuleFeatures' | 'buildConfig' | 'uidStrategy'>,
 ) {
   return createExtensionSpecification({
     identifier: spec.identifier,
     schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
     appModuleFeatures: spec.appModuleFeatures,
+    uidStrategy: spec.uidStrategy,
     buildConfig: spec.buildConfig ?? {mode: 'none'},
     deployConfig: async (config, directory) => {
       let parsedConfig = configWithoutFirstClassFields(config)

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

@@ -33,7 +33,7 @@ import {
   DeveloperPlatformClient,
   selectDeveloperPlatformClient,
 } from '../utilities/developer-platform-client.js'
-import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js'
+
 import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
 import {isServiceAccount, isUserAccount} from '@shopify/cli-kit/node/session'
 import {afterEach, beforeAll, beforeEach, describe, expect, test, vi} from 'vitest'
@@ -146,10 +146,7 @@ const mockTomlFileRemove = vi.fn()
 
 beforeAll(async () => {
   const localSpecs = await loadSpecifications.loadLocalExtensionsSpecifications()
-  const mockedRemoteSpecs = localSpecs.map((spec) => ({
-    ...spec,
-    loadedRemoteSpecs: true,
-  })) as RemoteAwareExtensionSpecification[]
+  const mockedRemoteSpecs = localSpecs.map((spec) => spec.markAsRemoteLoaded())
   vi.mocked(fetchSpecifications).mockResolvedValue(mockedRemoteSpecs)
 })
 

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

@@ -51,7 +51,7 @@ describe('generate', () => {
 
   beforeEach(async () => {
     const allSpecs = await loadLocalExtensionsSpecifications()
-    specifications = allSpecs.map((spec) => spec as RemoteAwareExtensionSpecification)
+    specifications = allSpecs.map((spec) => spec.markAsRemoteLoaded())
     developerPlatformClient = testDeveloperPlatformClient()
   })
 

packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts

@@ -50,7 +50,6 @@ describe('fetchExtensionSpecifications', () => {
     expect(got).toEqual(
       expect.arrayContaining([
         expect.objectContaining({
-          name: 'Product Subscription',
           externalName: 'Subscription UI',
           identifier: 'product_subscription',
           externalIdentifier: 'product_subscription_external',
@@ -63,12 +62,10 @@ describe('fetchExtensionSpecifications', () => {
     expect(got).toEqual(
       expect.arrayContaining([
         expect.objectContaining({
-          name: 'Online Store - App Theme Extension',
           externalName: 'Theme App Extension',
           identifier: 'theme',
           externalIdentifier: 'theme_external',
           registrationLimit: 1,
-          surface: undefined,
         }),
       ]),
     )

packages/app/src/cli/services/generate/fetch-extension-specifications.ts

@@ -35,34 +35,29 @@ export async function fetchSpecifications({
 }: FetchSpecificationsOptions): Promise<RemoteAwareExtensionSpecification[]> {
   const result: RemoteSpecification[] = await developerPlatformClient.specifications(app)
 
-  const extensionSpecifications: FlattenedRemoteSpecification[] = result
+  const remoteSpecifications: FlattenedRemoteSpecification[] = result
     .filter((specification) => ['extension', 'configuration'].includes(specification.experience))
     .map((spec) => {
-      const newSpec = spec as FlattenedRemoteSpecification
       // WORKAROUND: The identifiers in the API are different for these extensions to the ones the CLI
       // has been using so far. This is a workaround to keep the CLI working until the API is updated.
       if (spec.identifier === 'theme_app_extension') spec.identifier = 'theme'
       if (spec.identifier === 'subscription_management') spec.identifier = 'product_subscription'
-      newSpec.registrationLimit = spec.options.registrationLimit
-      newSpec.surface = spec.features?.argo?.surface
-
-      // Hardcoded value for the post purchase extension because the value is wrong in the API
-      if (spec.identifier === 'checkout_post_purchase') newSpec.surface = 'post_purchase'
-
-      return newSpec
+      return {
+        ...spec,
+        registrationLimit: spec.options.registrationLimit,
+        surface: spec.features?.argo?.surface,
+      }
     })
 
   const local = await loadLocalExtensionsSpecifications()
-  const updatedSpecs = await mergeLocalAndRemoteSpecs(local, extensionSpecifications)
+  const updatedSpecs = await mergeLocalAndRemoteSpecs(local, remoteSpecifications)
   return [...updatedSpecs]
 }
 
 async function mergeLocalAndRemoteSpecs(
   local: ExtensionSpecification[],
   remote: FlattenedRemoteSpecification[],
 ): Promise<RemoteAwareExtensionSpecification[]> {
-  // Iterate over the remote specs and merge them with the local ones
-  // If the local spec is missing, and the remote one has a validation schema, create a new local spec using contracts
   const updated = remote.map(async (remoteSpec) => {
     let localSpec = local.find((local) => local.identifier === remoteSpec.identifier)
     if (!localSpec && remoteSpec.validationSchema?.jsonSchema) {
@@ -71,17 +66,15 @@ async function mergeLocalAndRemoteSpecs(
       localSpec = createContractBasedModuleSpecification({
         identifier: remoteSpec.identifier,
         appModuleFeatures: () => (hasLocalization ? ['localization'] : []),
+        uidStrategy: remoteSpec.options.uidIsClientProvided ? 'uuid' : 'single',
       })
-      localSpec.uidStrategy = remoteSpec.options.uidIsClientProvided ? 'uuid' : 'single'
     }
     if (!localSpec) return undefined
 
-    const merged = {...localSpec, ...remoteSpec, loadedRemoteSpecs: true} as RemoteAwareExtensionSpecification &
-      FlattenedRemoteSpecification
+    const remoteAwareSpec = localSpec.applyRemoteSpecification(remoteSpec)
 
-    // If configuration is inside an app.toml -- i.e. single UID mode -- we must be able to parse a partial slice.
     let handleInvalidAdditionalProperties: HandleInvalidAdditionalProperties
-    switch (merged.uidStrategy) {
+    switch (remoteAwareSpec.uidStrategy) {
       case 'uuid':
         handleInvalidAdditionalProperties = 'fail'
         break
@@ -93,12 +86,13 @@ async function mergeLocalAndRemoteSpecs(
         break
     }
 
-    const parseConfigurationObject = await unifiedConfigurationParserFactory(merged, handleInvalidAdditionalProperties)
+    remoteAwareSpec.parseConfigurationObject = await unifiedConfigurationParserFactory(
+      remoteAwareSpec,
+      remoteSpec,
+      handleInvalidAdditionalProperties,
+    )
 
-    return {
-      ...merged,
-      parseConfigurationObject,
-    }
+    return remoteAwareSpec
   })
 
   const result = getArrayRejectingUndefined<RemoteAwareExtensionSpecification>(await Promise.all(updated))

packages/app/src/cli/utilities/json-schema.test.ts

@@ -1,4 +1,6 @@
 import {unifiedConfigurationParserFactory} from './json-schema.js'
+import {ExtensionSpecification} from '../models/extensions/specification.js'
+import {FlattenedRemoteSpecification} from '../api/graphql/extension_specifications.js'
 import {describe, test, expect} from 'vitest'
 import {randomUUID} from '@shopify/cli-kit/node/crypto'
 
@@ -18,19 +20,23 @@ describe('unifiedConfigurationParserFactory', () => {
     }
   }
 
-  test('falls back to zod parser when no JSON schema is provided', async () => {
-    // Given
-    const merged = {
+  function buildTestInputs(validationSchema?: {jsonSchema: string} | undefined | null) {
+    const spec = {
       identifier: randomUUID(),
       parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: undefined,
-    }
+    } as unknown as ExtensionSpecification & {loadedRemoteSpecs: true}
+    const remoteSpec = {
+      validationSchema,
+    } as unknown as FlattenedRemoteSpecification
+    return {spec, remoteSpec}
+  }
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+  test('falls back to zod parser when no JSON schema is provided', async () => {
+    const {spec, remoteSpec} = buildTestInputs(undefined)
+
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'product_subscription'})
 
-    // Then
     expect(result).toEqual({
       state: 'ok',
       data: {type: 'product_subscription'},
@@ -39,20 +45,11 @@ describe('unifiedConfigurationParserFactory', () => {
   })
 
   test('falls back to zod parser when JSON schema is empty', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({jsonSchema: '{}'})
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'product_subscription'})
 
-    // Then
     expect(result).toEqual({
       state: 'ok',
       data: {type: 'product_subscription'},
@@ -61,20 +58,13 @@ describe('unifiedConfigurationParserFactory', () => {
   })
 
   test('validates with both zod and JSON schema when both succeed', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}}}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({
+      jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}}}',
+    })
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'product_subscription'})
 
-    // Then
     expect(result).toEqual({
       state: 'ok',
       data: {type: 'product_subscription'},
@@ -83,41 +73,27 @@ describe('unifiedConfigurationParserFactory', () => {
   })
 
   test('returns errors when zod validation fails', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}}}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({
+      jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}}}',
+    })
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'invalid'})
 
-    // Then
     expect(result.state).toBe('error')
     expect(result.data).toBeUndefined()
     expect(result.errors).toHaveLength(1)
     expect(result.errors?.[0]).toEqual({path: ['type'], message: 'Invalid type'})
   })
 
   test('returns errors when JSON schema validation fails', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}},"required":["price"]}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({
+      jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}},"required":["price"]}',
+    })
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'product_subscription'})
 
-    // Then
     expect(result.state).toBe('error')
     expect(result.data).toBeUndefined()
     expect(result.errors).toBeDefined()
@@ -126,20 +102,13 @@ describe('unifiedConfigurationParserFactory', () => {
   })
 
   test('combines errors from both validations', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}},"required":["price"]}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({
+      jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}},"required":["price"]}',
+    })
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
     const result = parser({type: 'invalid'})
 
-    // Then
     expect(result.state).toBe('error')
     expect(result.data).toBeUndefined()
     expect(result.errors).toBeDefined()
@@ -153,19 +122,13 @@ describe('unifiedConfigurationParserFactory', () => {
   })
 
   test('adds base properties to the JSON schema', async () => {
-    // Given
-    const merged = {
-      identifier: randomUUID(),
-      parseConfigurationObject: mockParseConfigurationObject,
-      validationSchema: {
-        jsonSchema: '{"type":"object","properties":{"custom":{"type":"string"}}}',
-      },
-    }
+    const {spec, remoteSpec} = buildTestInputs({
+      jsonSchema: '{"type":"object","properties":{"custom":{"type":"string"}}}',
+    })
 
-    // When
-    const parser = await unifiedConfigurationParserFactory(merged as any)
+    const parser = await unifiedConfigurationParserFactory(spec, remoteSpec)
 
-    // Then - base properties should be accepted
+    // base properties should be accepted
     const result = parser({
       type: 'product_subscription',
       handle: 'test-handle',