From e801ce0565e2cb5b6adf69d45dff798fb56d36cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 7 Oct 2024 17:11:58 +0200 Subject: [PATCH 1/2] Extract config clean up to a single place --- packages/app/src/cli/models/app/loader.ts | 9 ++++---- .../cli/models/extensions/specification.ts | 22 +++++++++++++++---- .../src/cli/services/dev/update-extension.ts | 5 ++--- packages/app/src/cli/utilities/json-schema.ts | 6 ++--- 4 files changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index 731a0a04f5..52ccdf6947 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -529,20 +529,19 @@ class AppLoader, 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(schema: zod.ZodType, 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 +} diff --git a/packages/app/src/cli/services/dev/update-extension.ts b/packages/app/src/cli/services/dev/update-extension.ts index 8b253d0e0d..36398c1271 100644 --- a/packages/app/src/cli/services/dev/update-extension.ts +++ b/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( diff --git a/packages/app/src/cli/utilities/json-schema.ts b/packages/app/src/cli/utilities/json-schema.ts index 2063c0d001..fc89f645fe 100644 --- a/packages/app/src/cli/utilities/json-schema.ts +++ b/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 From e413bf41623802b0ecb1fb4b47379eec337f253b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 7 Oct 2024 17:35:28 +0200 Subject: [PATCH 2/2] add tests to configWithoutFirstClassFields --- .../specification.integration.test.ts | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/extensions/specification.integration.test.ts b/packages/app/src/cli/models/extensions/specification.integration.test.ts index b2a2abc941..9c1aab5bea 100644 --- a/packages/app/src/cli/models/extensions/specification.integration.test.ts +++ b/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', + }) + }) +})