From 509c9def10af154ca9848b0e02528d2b837dce54 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 19 Aug 2026 21:42:11 +0300 Subject: [PATCH] Add a default dev-session message for configuration-only extensions Config-only extension modules say nothing during `shopify app dev`: `logExtensionEvents` only runs inside `processEvents`, so at startup a module that is live on the platform is never mentioned in the terminal. Add a default `getDevSessionUpdateMessages` on `ExtensionInstance`, gated on a capability predicate (no features, no deploy steps, no build output, not app config) and on the first successful dev session. A per-spec hook still wins, so any specification can override the copy. The default lives on the instance rather than in the spec factories: those pass `getDevSessionUpdateMessages` through unconditionally, so an explicit `undefined` clobbers a factory-level default at `{...defaults, ...spec}`. `DevSessionResult` composes its success branch from the exported `DevSessionUpdateStatus` union rather than restating the literals, so the service and the spec hook can't drift apart. The logger takes the result itself and decides which outcomes are worth speaking about, so the status is never rewritten at a call site where it could disagree with the branch it sits in. Only the narrow context reaches the specs. Drop the `'aborted'` status while here. Nothing has constructed it since 4bc22d2c8c ("Only one app-preview update") removed the bundle-controller abort check, so the branch handling it was unreachable. Deleting it lets `DevSessionResult` reuse `DevSessionUpdateStatus` whole instead of grafting a third literal onto it. Co-Authored-By: Claude Opus 5 --- .changeset/nervous-pandas-acknowledge.md | 5 + .../extensions/extension-instance.test.ts | 203 +++++++++++++++++- .../models/extensions/extension-instance.ts | 26 ++- .../cli/models/extensions/specification.ts | 10 +- .../app_config_app_access.test.ts | 18 +- .../app_config_app_proxy.test.ts | 4 +- .../dev-session/dev-session-logger.test.ts | 105 ++++++++- .../dev-session/dev-session-logger.ts | 10 +- .../dev/processes/dev-session/dev-session.ts | 11 +- .../fetch-extension-specifications.test.ts | 90 ++++++++ 10 files changed, 451 insertions(+), 31 deletions(-) create mode 100644 .changeset/nervous-pandas-acknowledge.md diff --git a/.changeset/nervous-pandas-acknowledge.md b/.changeset/nervous-pandas-acknowledge.md new file mode 100644 index 00000000000..5d818319278 --- /dev/null +++ b/.changeset/nervous-pandas-acknowledge.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': minor +--- + +Show a confirmation when configuration-only extensions are accepted during `shopify app dev` diff --git a/packages/app/src/cli/models/extensions/extension-instance.test.ts b/packages/app/src/cli/models/extensions/extension-instance.test.ts index 99191649959..25672f501f3 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -1,5 +1,13 @@ import {SingleWebhookSubscriptionType} from './specifications/app_config_webhook_schemas/webhooks_schema.js' -import {MAX_EXTENSION_HANDLE_LENGTH} from './schemas.js' +import {DEFAULT_DEV_SESSION_UPDATE_MESSAGE, ExtensionInstance} from './extension-instance.js' +import { + ExtensionSpecification, + createConfigExtensionSpecification, + createContractBasedModuleSpecification, + createExtensionSpecification, +} from './specification.js' +import {loadLocalExtensionsSpecifications} from './load-specifications.js' +import {BaseConfigType, BaseSchema, MAX_EXTENSION_HANDLE_LENGTH} from './schemas.js' import {FunctionConfigType} from './specifications/function.js' import { testApp, @@ -15,6 +23,7 @@ import { placeholderAppConfiguration, } from '../app/app.test-data.js' import {ExtensionBuildOptions} from '../../services/build/extension.js' +import {ClientSteps} from '../../services/build/client-steps.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {joinPath} from '@shopify/cli-kit/node/path' import {describe, expect, test, vi} from 'vitest' @@ -714,3 +723,195 @@ describe('SHOPIFY_CLI_DISABLE_IMPORT_SCANNING', () => { }) }) }) + +describe('getDevSessionUpdateMessages', () => { + const deployStep: ClientSteps = [ + {lifecycle: 'deploy', steps: [{id: 'build-theme', name: 'Build Theme', type: 'build_theme'}]}, + ] + + function instanceFor(specification: ExtensionSpecification): ExtensionInstance { + return new ExtensionInstance({ + configuration: {name: 'test extension', type: specification.identifier} as BaseConfigType, + configurationPath: '', + directory: '/tmp/test-extension', + specification, + }) + } + + function specWithNoLocalDevOutput(identifier = 'no_local_dev_output') { + return createExtensionSpecification({identifier, schema: BaseSchema, appModuleFeatures: () => []}) + } + + test('returns the default message for a module with no local dev output on the first dev session', async () => { + const extensionInstance = instanceFor(specWithNoLocalDevOutput()) + + const got = await extensionInstance.getDevSessionUpdateMessages({status: 'created'}) + + expect(got).toEqual([DEFAULT_DEV_SESSION_UPDATE_MESSAGE]) + }) + + test('returns nothing for a module with no local dev output on subsequent updates', async () => { + const extensionInstance = instanceFor(specWithNoLocalDevOutput()) + + const got = await extensionInstance.getDevSessionUpdateMessages({status: 'updated'}) + + expect(got).toBeUndefined() + }) + + test('returns nothing when the module contributes features', async () => { + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'has_features', + schema: BaseSchema, + appModuleFeatures: () => ['localization'], + }), + ) + + expect(extensionInstance.hasNoLocalDevOutput).toBe(false) + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + test('returns nothing when the module has deploy steps', async () => { + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'has_deploy_steps', + schema: BaseSchema, + appModuleFeatures: () => [], + clientSteps: deployStep, + }), + ) + + expect(extensionInstance.hasNoLocalDevOutput).toBe(false) + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + test('returns nothing when the module produces build output', async () => { + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'has_build_output', + schema: BaseSchema, + appModuleFeatures: () => [], + getOutputRelativePath: () => 'dist/main.js', + }), + ) + + expect(extensionInstance.hasNoLocalDevOutput).toBe(false) + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + test('returns nothing for app config modules, which are summarised together instead', async () => { + const extensionInstance = instanceFor( + createConfigExtensionSpecification({ + identifier: 'app_config_without_messages', + schema: BaseSchema, + transformConfig: {}, + }), + ) + + expect(extensionInstance.isAppConfigExtension).toBe(true) + expect(extensionInstance.hasNoLocalDevOutput).toBe(true) + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + test('evaluates the app config exclusion lazily, so a remotely-rewritten experience is respected', async () => { + const specification = specWithNoLocalDevOutput() + const extensionInstance = instanceFor({...specification, experience: 'configuration'}) + + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + describe('per-spec override', () => { + const override = async () => ['Custom message'] + + test('wins over the default through createExtensionSpecification', async () => { + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'override_via_extension_spec', + schema: BaseSchema, + appModuleFeatures: () => [], + getDevSessionUpdateMessages: override, + }), + ) + + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toEqual([ + 'Custom message', + ]) + }) + + test('wins over the default through createConfigExtensionSpecification', async () => { + const extensionInstance = instanceFor( + createConfigExtensionSpecification({ + identifier: 'override_via_config_spec', + schema: BaseSchema, + transformConfig: {}, + getDevSessionUpdateMessages: override, + }), + ) + + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toEqual([ + 'Custom message', + ]) + }) + + test('wins over the default when spread onto a contract based module specification', async () => { + const specification = createContractBasedModuleSpecification({ + identifier: 'override_via_contract_based_spec', + experience: 'extension', + uidStrategy: 'single', + appModuleFeatures: () => [], + }) + const extensionInstance = instanceFor({...specification, getDevSessionUpdateMessages: override}) + + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'created'})).resolves.toEqual([ + 'Custom message', + ]) + }) + + test('is used even on subsequent updates, where the default stays quiet', async () => { + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'override_on_update', + schema: BaseSchema, + appModuleFeatures: () => [], + getDevSessionUpdateMessages: override, + }), + ) + + await expect(extensionInstance.getDevSessionUpdateMessages({status: 'updated'})).resolves.toEqual([ + 'Custom message', + ]) + }) + + test('receives the dev session context', async () => { + const getDevSessionUpdateMessages = vi.fn().mockResolvedValue([]) + const extensionInstance = instanceFor( + createExtensionSpecification({ + identifier: 'override_receiving_context', + schema: BaseSchema, + appModuleFeatures: () => [], + getDevSessionUpdateMessages, + }), + ) + + await extensionInstance.getDevSessionUpdateMessages({status: 'created'}) + + expect(getDevSessionUpdateMessages).toHaveBeenCalledWith(extensionInstance.configuration, {status: 'created'}) + }) + }) + + describe('local specifications matching the default', () => { + test('only modules with no local dev output receive the default message', async () => { + const specifications = await loadLocalExtensionsSpecifications() + + const matching = specifications + .filter((specification) => { + const extensionInstance = instanceFor(specification) + return !extensionInstance.isAppConfigExtension && extensionInstance.hasNoLocalDevOutput + }) + .map((specification) => specification.identifier) + .sort() + + expect(matching).toEqual(['editor_extension_collection', 'flow_action', 'flow_trigger', 'payments_extension']) + }) + }) +}) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 23ae5cb345c..cce10baee56 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -1,6 +1,11 @@ import {BaseConfigType, MAX_EXTENSION_HANDLE_LENGTH, MAX_UID_LENGTH} from './schemas.js' import {FunctionConfigType} from './specifications/function.js' -import {DevSessionWatchConfig, ExtensionFeature, ExtensionSpecification} from './specification.js' +import { + DevSessionUpdateContext, + DevSessionWatchConfig, + ExtensionFeature, + ExtensionSpecification, +} from './specification.js' import {SingleWebhookSubscriptionType} from './specifications/app_config_webhook_schemas/webhooks_schema.js' import {ExtensionBuildOptions} from '../../services/build/extension.js' import {ExtensionUuidsByLocalIdentifier} from '../app/identifiers.js' @@ -39,6 +44,8 @@ const DEFAULT_WATCH_IGNORE = [ '**/.gitignore', ] +export const DEFAULT_DEV_SESSION_UPDATE_MESSAGE = 'Configuration accepted' + /** * Class that represents an instance of a local extension * Before creating this class we've validated that: @@ -143,6 +150,10 @@ export class ExtensionInstance { - if (!this.specification.getDevSessionUpdateMessages) return undefined - return this.specification.getDevSessionUpdateMessages(this.configuration) + async getDevSessionUpdateMessages(context: DevSessionUpdateContext): Promise { + if (this.specification.getDevSessionUpdateMessages) { + return this.specification.getDevSessionUpdateMessages(this.configuration, context) + } + + if (context.status !== 'created') return undefined + if (this.isAppConfigExtension) return undefined + if (!this.hasNoLocalDevOutput) return undefined + + return [DEFAULT_DEV_SESSION_UPDATE_MESSAGE] } /** diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index 0dc1825a0a1..65290be5c1f 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -61,6 +61,12 @@ interface ExtensionDeployConfigContext { appConfiguration: AppConfiguration } +export type DevSessionUpdateStatus = 'created' | 'updated' + +export interface DevSessionUpdateContext { + status: DevSessionUpdateStatus +} + /** * Extension specification with all the needed properties and methods to load an extension. */ @@ -91,7 +97,7 @@ export interface ExtensionSpecification, outputPath: string) => Promise hasExtensionPointTarget?(config: TConfiguration, target: string): boolean appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[] - getDevSessionUpdateMessages?: (config: TConfiguration) => Promise + getDevSessionUpdateMessages?: (config: TConfiguration, context: DevSessionUpdateContext) => Promise patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void /** @@ -271,7 +277,7 @@ export function createConfigExtensionSpecification ExtensionFeature[] transformConfig: TransformationConfig | CustomTransformationConfig uidStrategy?: UidStrategy - getDevSessionUpdateMessages?: (config: TConfiguration) => Promise + getDevSessionUpdateMessages?: (config: TConfiguration, context: DevSessionUpdateContext) => Promise patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void }): ExtensionSpecification { const appModuleFeatures = spec.appModuleFeatures ?? (() => []) diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_app_access.test.ts b/packages/app/src/cli/models/extensions/specifications/app_config_app_access.test.ts index 22ec0a97ecd..e514e8d3864 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_app_access.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_app_access.test.ts @@ -88,7 +88,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Access scopes auto-granted: read_products, write_products']) @@ -106,7 +106,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Access scopes auto-granted: write_orders, read_inventory']) @@ -121,7 +121,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['App has been installed']) @@ -140,7 +140,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Using legacy install flow - access scopes are not auto-granted']) @@ -159,7 +159,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Using legacy install flow - access scopes are not auto-granted']) @@ -178,7 +178,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Access scopes auto-granted: read_products, write_products']) @@ -194,7 +194,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['Using legacy install flow - access scopes are not auto-granted']) @@ -212,7 +212,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['App has been installed']) @@ -230,7 +230,7 @@ describe('app_config_app_access', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual(['App has been installed']) diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.test.ts b/packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.test.ts index 78ceb214c47..9cd9bb51bbd 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.test.ts @@ -100,7 +100,7 @@ describe('app_config_app_proxy', () => { } // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual([ @@ -114,7 +114,7 @@ describe('app_config_app_proxy', () => { const config: AppProxyConfigType = {} // When - const result = await spec.getDevSessionUpdateMessages!(config) + const result = await spec.getDevSessionUpdateMessages!(config, {status: 'created'}) // Then expect(result).toEqual([]) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts index bbe871c54ca..f00878f1a63 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts @@ -1,11 +1,40 @@ import {DevSessionLogger} from './dev-session-logger.js' import {UserError} from './dev-session.js' import {AppEvent, EventType} from '../../app-events/app-event-watcher.js' -import {ExtensionInstance} from '../../../../models/extensions/extension-instance.js' +import { + DEFAULT_DEV_SESSION_UPDATE_MESSAGE, + ExtensionInstance, +} from '../../../../models/extensions/extension-instance.js' +import {createExtensionSpecification} from '../../../../models/extensions/specification.js' +import {BaseConfigType, BaseSchema} from '../../../../models/extensions/schemas.js' import {describe, expect, test, vi, beforeEach} from 'vitest' import {JsonMapType} from '@shopify/cli-kit/node/toml' import {Writable} from 'stream' +function extensionWithNoLocalDevOutput(handle: string): ExtensionInstance { + const specification = createExtensionSpecification({ + identifier: handle.replace(/-/g, '_'), + schema: BaseSchema, + appModuleFeatures: () => [], + uidStrategy: 'single', + }) + return new ExtensionInstance({ + configuration: {name: handle, type: specification.identifier, handle} as BaseConfigType, + configurationPath: '', + directory: '/tmp/test-extension', + specification, + }) +} + +function eventForExtensions(extensions: ExtensionInstance[], type = EventType.Updated): AppEvent { + return { + app: {configuration: {}} as AppEvent['app'], + extensionEvents: extensions.map((extension) => ({type, extension})), + path: '', + startTime: [0, 0], + } +} + describe('DevSessionLogger', () => { let output: string[] let stdout: Writable @@ -166,10 +195,38 @@ describe('DevSessionLogger', () => { describe('logExtensionUpdateMessages', () => { test('does nothing when no event is provided', async () => { - await logger.logExtensionUpdateMessages() + await logger.logExtensionUpdateMessages(undefined, {status: 'created'}) expect(output).toMatchInlineSnapshot(`[]`) }) + test('does not consult extensions when the dev session errored', async () => { + const mockExtension = { + getDevSessionUpdateMessages: vi.fn().mockResolvedValue(['This would be logged if the session had succeeded.']), + entrySourceFilePath: '', + devUUID: '', + localIdentifier: '', + idEnvironmentVariableName: '', + handle: 'test-extension', + } as unknown as ExtensionInstance + + const event: AppEvent = { + app: {configuration: {}} as any, + extensionEvents: [ + { + type: 'updated' as EventType, + extension: mockExtension, + }, + ], + path: '', + startTime: [0, 0], + } + + await logger.logExtensionUpdateMessages(event, {status: 'remote-error', error: []}) + + expect(output).toMatchInlineSnapshot(`[]`) + expect(mockExtension.getDevSessionUpdateMessages).not.toHaveBeenCalled() + }) + test('logs messages', async () => { const mockExtension = { getDevSessionUpdateMessages: vi.fn().mockResolvedValue(['This has been updated.']), @@ -192,7 +249,7 @@ describe('DevSessionLogger', () => { startTime: [0, 0], } - await logger.logExtensionUpdateMessages(event) + await logger.logExtensionUpdateMessages(event, {status: 'created'}) expect(output).toMatchInlineSnapshot(` [ "└ This has been updated.", @@ -222,10 +279,50 @@ describe('DevSessionLogger', () => { startTime: [0, 0], } - await logger.logExtensionUpdateMessages(event) + await logger.logExtensionUpdateMessages(event, {status: 'created'}) expect(output).toMatchInlineSnapshot(`[]`) expect(mockExtension.getDevSessionUpdateMessages).not.toHaveBeenCalled() }) + + test('logs the default message for a module with no local dev output on the first dev session', async () => { + const event = eventForExtensions([extensionWithNoLocalDevOutput('analytics-app-events')]) + + await logger.logExtensionUpdateMessages(event, {status: 'created'}) + + expect(output).toEqual([`\u001b[90m\u2514 \u001b[39m${DEFAULT_DEV_SESSION_UPDATE_MESSAGE}`]) + }) + + test('logs nothing for a module with no local dev output on subsequent updates', async () => { + const event = eventForExtensions([extensionWithNoLocalDevOutput('analytics-app-events')]) + + await logger.logExtensionUpdateMessages(event, {status: 'updated'}) + + expect(output).toMatchInlineSnapshot(`[]`) + }) + + test('logs nothing for a deleted module with no local dev output', async () => { + const event = eventForExtensions([extensionWithNoLocalDevOutput('analytics-app-events')], EventType.Deleted) + + await logger.logExtensionUpdateMessages(event, {status: 'created'}) + + expect(output).toMatchInlineSnapshot(`[]`) + }) + + test('draws a single tree across all extensions, with the branch character on the last message', async () => { + const event = eventForExtensions([ + extensionWithNoLocalDevOutput('first-module'), + extensionWithNoLocalDevOutput('second-module'), + ]) + + await logger.logExtensionUpdateMessages(event, {status: 'created'}) + + expect(output).toMatchInlineSnapshot(` + [ + "│ Configuration accepted", + "└ Configuration accepted", + ] + `) + }) }) describe('logMultipleErrors', () => { diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts index add75295c6d..93bed1786ff 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts @@ -1,6 +1,7 @@ -import {UserError} from './dev-session.js' +import {DevSessionResult, UserError} from './dev-session.js' import {AppEvent, EventType} from '../../app-events/app-event-watcher.js' import {ExtensionInstance} from '../../../../models/extensions/extension-instance.js' +import {DevSessionUpdateContext} from '../../../../models/extensions/specification.js' import {outputToken, outputContent, outputDebug} from '@shopify/cli-kit/node/output' import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components' import {Writable} from 'stream' @@ -73,14 +74,17 @@ export class DevSessionLogger { * Display update messages from extensions after a dev session update. * This function collects and displays update messages from all extensions. */ - async logExtensionUpdateMessages(event?: AppEvent) { + async logExtensionUpdateMessages(event: AppEvent | undefined, result: DevSessionResult) { if (!event) return + // Errored sessions are reported through `logUserErrors`, which already speaks per extension. + if (result.status === 'remote-error' || result.status === 'unknown-error') return + const context: DevSessionUpdateContext = {status: result.status} const extensionEvents = event.extensionEvents ?? [] const messageArrays = await Promise.all( extensionEvents.map(async (eve) => { // Don't log messages for deleted extensions if (eve.type === EventType.Deleted) return [] - const messages = await eve.extension.getDevSessionUpdateMessages() + const messages = await eve.extension.getDevSessionUpdateMessages(context) return messages?.map((message) => ({message, prefix: eve.extension.handle})) ?? [] }), ) diff --git a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts index 6a6c15d68f7..dcf11015567 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session/dev-session.ts @@ -11,6 +11,7 @@ import { } from '../../../bundle.js' import {DevSessionCreateOptions, DevSessionUpdateOptions} from '../../../../utilities/developer-platform-client.js' import {AppManifest} from '../../../../models/app/app.js' +import {DevSessionUpdateStatus} from '../../../../models/extensions/specification.js' import {getWebSocketUrl} from '../../extension.js' import {endHRTimeInMs, startHRTime} from '@shopify/cli-kit/node/hrtime' import {ClientError} from 'graphql-request' @@ -35,8 +36,8 @@ interface DevSessionState { userEmail?: string | null } -type DevSessionResult = - | {status: 'updated' | 'created' | 'aborted'} +export type DevSessionResult = + | {status: DevSessionUpdateStatus} | {status: 'remote-error'; error: UserError[]} | {status: 'unknown-error'; error: Error} @@ -228,15 +229,13 @@ export class DevSession { private async handleDevSessionResult(result: DevSessionResult, event?: AppEvent) { if (result.status === 'updated') { await this.logger.success(`✅ Updated dev preview on ${this.options.storeFqdn}`) - await this.logger.logExtensionUpdateMessages(event) + await this.logger.logExtensionUpdateMessages(event, result) await this.setUpdatedStatusMessage() } else if (result.status === 'created') { this.statusManager.updateStatus({isReady: true}) await this.logger.success(`✅ Ready, watching for changes in your app `) - await this.logger.logExtensionUpdateMessages(event) + await this.logger.logExtensionUpdateMessages(event, result) this.statusManager.setMessage('READY') - } else if (result.status === 'aborted') { - await this.logger.debug('❌ Dev preview update aborted (new change detected or error during update)') } else if (result.status === 'remote-error' || result.status === 'unknown-error') { await this.logger.logUserErrors(result.error, event?.app.allExtensions ?? []) if (result.error instanceof Error && (result.error as Error & {cause?: string}).cause === 'validation-error') { 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..49c1e82bf9d 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 @@ -1,5 +1,9 @@ import {fetchSpecifications} from './fetch-extension-specifications.js' import {testDeveloperPlatformClient, testOrganizationApp} from '../../models/app/app.test-data.js' +import {RemoteSpecification} from '../../api/graphql/extension_specifications.js' +import {DEFAULT_DEV_SESSION_UPDATE_MESSAGE, ExtensionInstance} from '../../models/extensions/extension-instance.js' +import {BaseConfigType} from '../../models/extensions/schemas.js' +import {ExtensionSpecification} from '../../models/extensions/specification.js' import {describe, expect, test} from 'vitest' describe('fetchExtensionSpecifications', () => { @@ -107,3 +111,89 @@ describe('fetchExtensionSpecifications', () => { expect(withLocalization?.appModuleFeatures()).toEqual(['localization']) }) }) + +describe('getDevSessionUpdateMessages for remotely-sourced specifications', () => { + const analyticsAppEventsRemoteSpec: RemoteSpecification = { + name: 'Analytics App Events', + externalName: 'Analytics App Events', + identifier: 'analytics_app_events', + externalIdentifier: 'analytics_app_events_external', + gated: false, + experience: 'extension', + managementExperience: 'cli', + registrationLimit: 1, + uidStrategy: 'single', + validationSchema: { + jsonSchema: + '{"$schema":"http://json-schema.org/draft-07/schema#","type":"object","additionalProperties":false,"properties":{"name":{"type":"string"}}}', + }, + } + + function instanceFor(specification: ExtensionSpecification): ExtensionInstance { + return new ExtensionInstance({ + configuration: {name: 'analytics app events', type: specification.identifier} as BaseConfigType, + configurationPath: '', + directory: '/tmp/test-extension', + specification, + }) + } + + async function specificationsFor(remoteSpecs: RemoteSpecification[]): Promise { + return fetchSpecifications({ + developerPlatformClient: testDeveloperPlatformClient({specifications: () => Promise.resolve(remoteSpecs)}), + app: testOrganizationApp(), + }) + } + + test('a remote-only specification with no local dev output gets the default message', async () => { + // Given + const specifications = await specificationsFor([analyticsAppEventsRemoteSpec]) + const specification = specifications.find((spec) => spec.identifier === 'analytics_app_events')! + + // Then + expect(specification.experience).toBe('extension') + expect(specification.uidStrategy).toBe('single') + expect(specification.appModuleFeatures()).toEqual([]) + await expect(instanceFor(specification).getDevSessionUpdateMessages({status: 'created'})).resolves.toEqual([ + DEFAULT_DEV_SESSION_UPDATE_MESSAGE, + ]) + }) + + test('a remote-only specification with localization does not get the default message', async () => { + // A known, accepted consequence of the predicate: `localization` counts as a feature. + const specifications = await specificationsFor([ + { + ...analyticsAppEventsRemoteSpec, + identifier: 'remote_only_with_localization', + validationSchema: { + jsonSchema: + '{"$schema":"http://json-schema.org/draft-07/schema#","type":"object","additionalProperties":false,"properties":{"localization":{"type":"object"}}}', + }, + }, + ]) + const specification = specifications.find((spec) => spec.identifier === 'remote_only_with_localization')! + + expect(specification.appModuleFeatures()).toEqual(['localization']) + await expect(instanceFor(specification).getDevSessionUpdateMessages({status: 'created'})).resolves.toBeUndefined() + }) + + test('merging a local specification with its remote counterpart preserves a per-specification override', async () => { + const specifications = await specificationsFor([ + { + ...analyticsAppEventsRemoteSpec, + name: 'App Home', + identifier: 'app_home', + externalIdentifier: 'app_home_external', + experience: 'configuration', + }, + ]) + const specification = specifications.find((spec) => spec.identifier === 'app_home')! + + expect(specification.getDevSessionUpdateMessages).toBeDefined() + await expect( + specification.getDevSessionUpdateMessages!({application_url: 'https://example.com'} as BaseConfigType, { + status: 'created', + }), + ).resolves.toEqual(['Using URL: https://example.com']) + }) +})