Merge pull request #4588 from Shopify/improve-first-class-types

Extract config clean up to a single place

packages/app/src/cli/models/app/loader.ts

@@ -529,20 +529,19 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
  const mergedConfig = {...configuration, ...extensionConfig}
 
  // Remove `extensions` and `path`, they are injected automatically but not needed nor expected by the contract
- const {extensions, path, ...restConfig} = mergedConfig
- if (!restConfig.handle) {
+ if (!mergedConfig.handle) {
  // Handle is required for unified config extensions.
  this.abortOrReport(
- outputContent`Missing handle for extension "${restConfig.name}" at ${relativePath(
+ outputContent`Missing handle for extension "${mergedConfig.name}" at ${relativePath(
  appDirectory,
  configurationPath,
  )}`,
  undefined,
  configurationPath,
  )
- restConfig.handle = 'unknown-handle'
+ mergedConfig.handle = 'unknown-handle'
  }
- return this.createExtensionInstance(mergedConfig.type, restConfig, configurationPath, directory)
+ return this.createExtensionInstance(mergedConfig.type, mergedConfig, configurationPath, directory)
  })
  return Promise.all(extensionsInstancesPromises)
  } else if (type) {

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

@@ -1,5 +1,5 @@
 import {loadLocalExtensionsSpecifications} from './load-specifications.js'
-import {createContractBasedModuleSpecification} from './specification.js'
+import {configWithoutFirstClassFields, createContractBasedModuleSpecification} from './specification.js'
 import {AppSchema} from '../app/app.js'
 import {describe, test, expect, beforeAll} from 'vitest'
 
@@ -44,3 +44,28 @@ describe('createContractBasedModuleSpecification', () => {
  expect(got.appModuleFeatures()).toEqual(['bundling'])
  })
 })
+
+describe('configWithoutFirstClassFields', () => {
+ test('removes the first class fields from the config', () => {
+ // When
+ const got = configWithoutFirstClassFields({
+ type: 'test',
+ handle: 'test',
+ uid: 'test',
+ path: 'test',
+ extensions: [{type: 'test', handle: 'test', uid: 'test', path: 'test'}],
+ config: {
+ test: 'test',
+ },
+ other: 'other',
+ })
+
+ // Then
+ expect(got).toEqual({
+ config: {
+ test: 'test',
+ },
+ other: 'other',
+ })
+ })
+})

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

@@ -8,6 +8,7 @@ import {Result} from '@shopify/cli-kit/node/result'
 import {capitalize} from '@shopify/cli-kit/common/string'
 import {ParseConfigurationResult, zod} from '@shopify/cli-kit/node/schema'
 import {getPathValue, setPathValue} from '@shopify/cli-kit/common/object'
+import {JsonMapType} from '@shopify/cli-kit/node/toml'
 
 export type ExtensionFeature =
  | 'ui_preview'
@@ -272,10 +273,7 @@ export function createContractBasedModuleSpecification<TConfiguration extends Ba
  schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
  appModuleFeatures: () => appModuleFeatures ?? [],
  deployConfig: async (config, _) => {
- // These are loaded automatically for all modules, but are considered "first class" and not part of extension contracts
- // If a module needs them, they can access them from the manifest.
- const {type, handle, uid, ...configWithoutFirstClassFields} = config
- return configWithoutFirstClassFields
+ return configWithoutFirstClassFields(config)
  },
  })
 }
@@ -405,3 +403,19 @@ function defaultAppConfigReverseTransform<T>(schema: zod.ZodType<T, any, any>, c
  return result
  }, {})
 }
+
+/**
+ * Remove the first class fields from the config.
+ *
+ * These are the fields that are not part of the specific extension contract, but are added automatically to all modules tomls.
+ * So it must be cleaned before validation or deployment.
+ * (this was initially done automatically by zod, but is not supported by JSON Schema & contracts)
+ *
+ * @param config - The config to remove the first class fields from
+ *
+ * @returns The config without the first class fields
+ */
+export function configWithoutFirstClassFields(config: JsonMapType): JsonMapType {
+ const {type, handle, uid, path, extensions, ...configWithoutFirstClassFields} = config
+ return configWithoutFirstClassFields
+}

packages/app/src/cli/services/dev/update-extension.ts

@@ -7,6 +7,7 @@ import {
 } from '../../models/app/loader.js'
 import {ExtensionInstance} from '../../models/extensions/extension-instance.js'
 import {ExtensionsArraySchema, UnifiedSchema} from '../../models/extensions/schemas.js'
+import {configWithoutFirstClassFields} from '../../models/extensions/specification.js'
 import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
 import {themeExtensionConfig} from '../deploy/theme-extension-config.js'
 import {AbortError} from '@shopify/cli-kit/node/error'
@@ -106,9 +107,7 @@ export async function reloadExtensionConfig({extension}: UpdateExtensionConfigOp
  }
 
  const mergedConfig = {...configuration, ...extensionConfig}
- // Remove `extensions` and `path`, they are injected automatically but not needed nor expected by the contract
- const {extensions, path, ...restConfig} = mergedConfig
- configObject = restConfig
+ configObject = configWithoutFirstClassFields(mergedConfig)
  }
 
  const newConfig = await parseConfigurationObjectAgainstSpecification(

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

@@ -1,6 +1,6 @@
 import {FlattenedRemoteSpecification} from '../api/graphql/extension_specifications.js'
 import {BaseConfigType} from '../models/extensions/schemas.js'
-import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js'
+import {configWithoutFirstClassFields, RemoteAwareExtensionSpecification} from '../models/extensions/specification.js'
 import {ParseConfigurationResult} from '@shopify/cli-kit/node/schema'
 import {jsonSchemaValidate, normaliseJsonSchema} from '@shopify/cli-kit/node/json-schema'
 import {isEmpty} from '@shopify/cli-kit/common/object'
@@ -28,9 +28,7 @@ export async function unifiedConfigurationParserFactory(
  // eslint-disable-next-line @typescript-eslint/no-explicit-any
  const subjectForAjv = zodValidatedData ?? (config as any)
 
- // These are loaded automatically for all modules, but are considered "first class" and not part of extension contracts
- // If a module needs them, they can access them from the manifest.
- const {type, handle, uid, ...subjectForAjvWithoutFirstClassFields} = subjectForAjv
+ const subjectForAjvWithoutFirstClassFields = configWithoutFirstClassFields(subjectForAjv)
  const jsonSchemaParse = jsonSchemaValidate(subjectForAjvWithoutFirstClassFields, contract)
 
  // Finally, we de-duplicate the error set from both validations -- identical messages for identical paths are removed