diff --git a/.changeset/config-contract-root-required.md b/.changeset/config-contract-root-required.md new file mode 100644 index 00000000000..953d8f9d6e4 --- /dev/null +++ b/.changeset/config-contract-root-required.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli': patch +--- + +Fix "[events]: Required" validation errors for apps whose configuration has no [events] section diff --git a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts index f78346c1a2f..713b68f3027 100644 --- a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts @@ -192,4 +192,9 @@ describe('transformToEventsConfig', () => { }, }) }) + + test('omits the events section when there is no events config', () => { + expect(transformToEventsConfig({})).toEqual({}) + expect(transformToEventsConfig({events: {}})).toEqual({}) + }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts index da5883ce1de..1a5a3646789 100644 --- a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts +++ b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts @@ -54,8 +54,11 @@ export function transformToEventsConfig(content: object) { return rest }) - const events = - (apiVersion ?? cleanedSubscriptions) ? {api_version: apiVersion, subscription: cleanedSubscriptions} : {} + // Omit the section entirely when there is no events config, so an empty `events` object + // isn't injected into linked configurations or deploy diffs + if (apiVersion === undefined && cleanedSubscriptions === undefined) { + return {} + } - return {events} + return {events: {api_version: apiVersion, subscription: cleanedSubscriptions}} } diff --git a/packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts b/packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts index 189242534d7..cc36bbbfa03 100644 --- a/packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts +++ b/packages/app/src/cli/services/generate/fetch-extension-specifications.test.ts @@ -106,4 +106,33 @@ describe('fetchExtensionSpecifications', () => { expect(withoutLocalization?.appModuleFeatures()).toEqual([]) expect(withLocalization?.appModuleFeatures()).toEqual(['localization']) }) + + test('configuration-experience specs do not require their section to be present in the app config', async () => { + // Given - a configuration-style spec whose remote contract has a root-level required property + const got = await fetchSpecifications({ + developerPlatformClient: testDeveloperPlatformClient(), + app: testOrganizationApp(), + }) + const configStyleSpec = got.find((spec) => spec.identifier === 'remote_only_extension_schema_config_style') + + // When - parsing an app config that doesn't include the spec's section + const resultWithoutSection = configStyleSpec?.parseConfigurationObject({name: 'my app'}) + + // Then - the root-level required property is not enforced + expect(resultWithoutSection).toEqual({ + state: 'ok', + data: {name: 'my app'}, + errors: undefined, + }) + + // When - parsing an app config where the section has the wrong type + const resultWithInvalidSection = configStyleSpec?.parseConfigurationObject({name: 'my app', pattern: 123}) + + // Then - the rest of the contract is still enforced + expect(resultWithInvalidSection?.state).toBe('error') + expect(resultWithInvalidSection?.errors).toContainEqual({ + path: ['pattern'], + message: 'Expected string, received number', + }) + }) }) diff --git a/packages/app/src/cli/utilities/json-schema.test.ts b/packages/app/src/cli/utilities/json-schema.test.ts index c139dc6e3de..2e7c74d143b 100644 --- a/packages/app/src/cli/utilities/json-schema.test.ts +++ b/packages/app/src/cli/utilities/json-schema.test.ts @@ -152,6 +152,72 @@ describe('unifiedConfigurationParserFactory', () => { expect(priceError).toBeDefined() }) + test('ignores root-level required properties for configuration-experience specs', async () => { + // Given + const merged = { + identifier: randomUUID(), + experience: 'configuration', + parseConfigurationObject: mockParseConfigurationObject, + validationSchema: { + jsonSchema: + '{"type":"object","properties":{"events":{"type":"object","properties":{"api_version":{"type":"string"}},"required":["api_version"]}},"required":["events"]}', + }, + } + + // When + const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema) + const result = parser({name: 'my app'}) + + // Then - the absent section must not be required, and non-contract keys are stripped + expect(result).toEqual({ + state: 'ok', + data: {}, + errors: undefined, + }) + }) + + test('still enforces nested required properties for configuration-experience specs when the section is present', async () => { + // Given + const merged = { + identifier: randomUUID(), + experience: 'configuration', + parseConfigurationObject: mockParseConfigurationObject, + validationSchema: { + jsonSchema: + '{"type":"object","properties":{"events":{"type":"object","properties":{"api_version":{"type":"string"}},"required":["api_version"]}},"required":["events"]}', + }, + } + + // When + const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema) + const result = parser({name: 'my app', events: {}}) + + // Then + expect(result.state).toBe('error') + expect(result.errors).toContainEqual({path: ['events', 'api_version'], message: 'Required'}) + expect(result.errors).not.toContainEqual({path: ['events'], message: 'Required'}) + }) + + test('still enforces root-level required properties for extension-experience specs', async () => { + // Given + const merged = { + identifier: randomUUID(), + experience: 'extension', + parseConfigurationObject: mockParseConfigurationObject, + validationSchema: { + jsonSchema: '{"type":"object","properties":{"type":{"type":"string"}},"required":["price"]}', + }, + } + + // When + const parser = await unifiedConfigurationParserFactory(merged as any, merged.validationSchema) + const result = parser({type: 'product_subscription'}) + + // Then + expect(result.state).toBe('error') + expect(result.errors).toContainEqual({path: ['price'], message: 'Required'}) + }) + test('adds base properties to the JSON schema', async () => { // Given const merged = { diff --git a/packages/app/src/cli/utilities/json-schema.ts b/packages/app/src/cli/utilities/json-schema.ts index 76ac3d5e411..90518a2db9d 100644 --- a/packages/app/src/cli/utilities/json-schema.ts +++ b/packages/app/src/cli/utilities/json-schema.ts @@ -41,6 +41,12 @@ export async function unifiedConfigurationParserFactory( } const contract = await normaliseJsonSchema(contractJsonSchema) contract.properties = {...JsonSchemaBaseProperties, ...contract.properties} + // Configuration-experience specs validate the entire app config; an absent section means "no module for this + // spec" (decided by the loader), so a root-level `required` from the contract must not force the section into + // existence. Nested `required` properties still apply once the section is present. + if (merged.experience === 'configuration') { + delete contract.required + } const extensionIdentifier = merged.identifier const parseConfigurationObject = (config: object): ParseConfigurationResult => {