Merge pull request #4532 from Shopify/fix-contract-schema-parsing

Clean up extension config
Shopify · Oct 1, 2024 · 90e17ce · 90e17ce
2 parents cd9c393 + 64fa18e
commit 90e17ce
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 5 deletions.
diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts
@@ -506,7 +506,9 @@ class AppLoader<TConfig extends AppConfiguration, TModuleSpec extends ExtensionS
         const configuration = await this.parseConfigurationFile(UnifiedSchema, configurationPath)
         const extensionsInstancesPromises = configuration.extensions.map(async (extensionConfig) => {
           const mergedConfig = {...configuration, ...extensionConfig}
-          const {extensions, ...restConfig} = mergedConfig
+
+          // Remove `extensions` and `path`, they are injected automatically but not needed nor expected by the contract
+          const {extensions, path, ...restConfig} = mergedConfig
           if (!restConfig.handle) {
             // Handle is required for unified config extensions.
             this.abortOrReport(

diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts
@@ -272,7 +272,10 @@ export function createContractBasedModuleSpecification<TConfiguration extends Ba
     schema: zod.any({}) as unknown as ZodSchemaType<TConfiguration>,
     appModuleFeatures: () => appModuleFeatures ?? [],
     deployConfig: async (config, _) => {
-      return 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
     },
   })
 }

diff --git a/packages/app/src/cli/services/dev/update-extension.ts b/packages/app/src/cli/services/dev/update-extension.ts
@@ -105,7 +105,10 @@ export async function reloadExtensionConfig({extension}: UpdateExtensionConfigOp
       )
     }
 
-    configObject = {...configuration, ...extensionConfig}
+    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
   }
 
   const newConfig = await parseConfigurationObjectAgainstSpecification(

diff --git a/packages/app/src/cli/utilities/json-schema.ts b/packages/app/src/cli/utilities/json-schema.ts
@@ -25,8 +25,13 @@ export async function unifiedConfigurationParserFactory(
 
     // Then, even if this failed, we try to validate against the contract.
     const zodValidatedData = zodParse.state === 'ok' ? zodParse.data : undefined
-    const subjectForAjv = zodValidatedData ?? config
-    const jsonSchemaParse = jsonSchemaValidate(subjectForAjv, contract)
+    // 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 jsonSchemaParse = jsonSchemaValidate(subjectForAjvWithoutFirstClassFields, contract)
 
     // Finally, we de-duplicate the error set from both validations -- identical messages for identical paths are removed
     let errors = zodParse.errors || []