Skip to content

Commit ebaf571

Browse files
nickwesselmannickwesselman
committed
simplify the changes
1 parent b24c909 commit ebaf571

File tree

7 files changed

+51
-31
lines changed

7 files changed

+51
-31
lines changed

‎packages/app/src/cli/models/extensions/extension-instance.ts‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import {Identifiers} from '../app/identifiers.js'
2525
import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
2626
import {AppConfigurationWithoutPath} from '../app/app.js'
2727
import {ApplicationURLs} from '../../services/dev/urls.js'
28-
import {EventType} from '../../services/dev/app-events/app-event-watcher.js'
2928
import {ok} from '@shopify/cli-kit/node/result'
3029
import {constantize, slugify} from '@shopify/cli-kit/common/string'
3130
import {hashString, nonRandomUUID} from '@shopify/cli-kit/node/crypto'
@@ -450,9 +449,9 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
450449
}
451450
}
452451

453-
async getDevSessionUpdateMessage(eventType: EventType): Promise<string[] | undefined> {
454-
if (!this.specification.getDevSessionUpdateMessage) return undefined
455-
return this.specification.getDevSessionUpdateMessage(this.configuration, eventType)
452+
async getDevSessionUpdateMessages(): Promise<string[] | undefined> {
453+
if (!this.specification.getDevSessionUpdateMessages) return undefined
454+
return this.specification.getDevSessionUpdateMessages(this.configuration)
456455
}
457456

458457
/**

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import {Flag} from '../../utilities/developer-platform-client.js'
77
import {AppConfigurationWithoutPath} from '../app/app.js'
88
import {loadLocalesConfig} from '../../utilities/extensions/locales-configuration.js'
99
import {ApplicationURLs} from '../../services/dev/urls.js'
10-
import {EventType} from '../../services/dev/app-events/app-event-watcher.js'
1110
import {Result} from '@shopify/cli-kit/node/result'
1211
import {capitalize} from '@shopify/cli-kit/common/string'
1312
import {ParseConfigurationResult, zod} from '@shopify/cli-kit/node/schema'
@@ -75,7 +74,7 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
7574
buildValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
7675
hasExtensionPointTarget?(config: TConfiguration, target: string): boolean
7776
appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[]
78-
getDevSessionUpdateMessage?: (config: TConfiguration, eventType: EventType) => Promise<string[]>
77+
getDevSessionUpdateMessages?: (config: TConfiguration) => Promise<string[]>
7978
patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void
8079

8180
/**
@@ -187,7 +186,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
187186
reverseTransform: spec.transformRemoteToLocal,
188187
experience: spec.experience ?? 'extension',
189188
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
190-
getDevSessionUpdateMessage: spec.getDevSessionUpdateMessage,
189+
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
191190
}
192191
const merged = {...defaults, ...spec}
193192

@@ -232,7 +231,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
232231
appModuleFeatures?: (config?: TConfiguration) => ExtensionFeature[]
233232
transformConfig: TransformationConfig | CustomTransformationConfig
234233
uidStrategy?: UidStrategy
235-
getDevSessionUpdateMessage?: (config: TConfiguration, eventType: EventType) => Promise<string[]>
234+
getDevSessionUpdateMessages?: (config: TConfiguration) => Promise<string[]>
236235
patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void
237236
}): ExtensionSpecification<TConfiguration> {
238237
const appModuleFeatures = spec.appModuleFeatures ?? (() => [])
@@ -246,7 +245,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
246245
transformRemoteToLocal: resolveReverseAppConfigTransform(spec.schema, spec.transformConfig),
247246
experience: 'configuration',
248247
uidStrategy: spec.uidStrategy ?? 'single',
249-
getDevSessionUpdateMessage: spec.getDevSessionUpdateMessage,
248+
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
250249
patchWithAppDevURLs: spec.patchWithAppDevURLs,
251250
})
252251
}

‎packages/app/src/cli/models/extensions/specifications/app_config_app_access.test.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ describe('app_config_app_access', () => {
7575
})
7676
})
7777

78-
describe('getDevSessionUpdateMessage', () => {
78+
describe('getDevSessionUpdateMessages', () => {
7979
test('should return message with scopes when scopes are provided', async () => {
8080
// Given
8181
const config = {
@@ -88,10 +88,10 @@ describe('app_config_app_access', () => {
8888
}
8989

9090
// When
91-
const result = await spec.getDevSessionUpdateMessage!(config)
91+
const result = await spec.getDevSessionUpdateMessages!(config)
9292

9393
// Then
94-
expect(result).toBe('Access scopes auto-granted: read_products, write_products')
94+
expect(result).toEqual(['Access scopes auto-granted: read_products, write_products'])
9595
})
9696

9797
test('should return message with required_scopes when only required_scopes are provided', async () => {
@@ -106,10 +106,10 @@ describe('app_config_app_access', () => {
106106
}
107107

108108
// When
109-
const result = await spec.getDevSessionUpdateMessage!(config)
109+
const result = await spec.getDevSessionUpdateMessages!(config)
110110

111111
// Then
112-
expect(result).toBe('Access scopes auto-granted: write_orders, read_inventory')
112+
expect(result).toEqual(['Access scopes auto-granted: write_orders, read_inventory'])
113113
})
114114

115115
test('should return default message when no scopes are provided', async () => {
@@ -121,10 +121,10 @@ describe('app_config_app_access', () => {
121121
}
122122

123123
// When
124-
const result = await spec.getDevSessionUpdateMessage!(config)
124+
const result = await spec.getDevSessionUpdateMessages!(config)
125125

126126
// Then
127-
expect(result).toBe('App has been installed')
127+
expect(result).toEqual(['App has been installed'])
128128
})
129129
})
130130
})

‎packages/app/src/cli/models/extensions/specifications/app_config_app_access.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ const appAccessSpec = createConfigExtensionSpecification({
4646
identifier: AppAccessSpecIdentifier,
4747
schema: AppAccessSchema,
4848
transformConfig: AppAccessTransformConfig,
49-
getDevSessionUpdateMessage: async (config) => {
49+
getDevSessionUpdateMessages: async (config) => {
5050
const scopesString = config.access_scopes?.scopes
5151
? config.access_scopes.scopes
5252
.split(',')

‎packages/app/src/cli/models/extensions/specifications/app_config_app_proxy.ts‎

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import {validateUrl} from '../../app/validation/common.js'
22
import {ExtensionSpecification, TransformationConfig, createConfigExtensionSpecification} from '../specification.js'
33
import {BaseSchema} from '../schemas.js'
4-
import {EventType} from '../../../services/dev/app-events/app-event-watcher.js'
54
import {zod} from '@shopify/cli-kit/node/schema'
65

76
const AppProxySchema = BaseSchema.extend({
@@ -26,6 +25,12 @@ const appProxySpec: ExtensionSpecification = createConfigExtensionSpecification(
2625
identifier: AppProxySpecIdentifier,
2726
schema: AppProxySchema,
2827
transformConfig: AppProxyTransformConfig,
28+
getDevSessionUpdateMessages: async (config) => {
29+
return [
30+
`Using app proxy URL: ${config.app_proxy?.url}`,
31+
`Note: Changes to app proxy prefix and subpath won't affect existing installations.`,
32+
]
33+
},
2934
patchWithAppDevURLs: (config, urls) => {
3035
if (urls.appProxy) {
3136
config.app_proxy = {
@@ -35,16 +40,6 @@ const appProxySpec: ExtensionSpecification = createConfigExtensionSpecification(
3540
}
3641
}
3742
},
38-
getDevSessionUpdateMessage: async (config, eventType) => {
39-
if (eventType === EventType.Deleted) {
40-
return []
41-
}
42-
const messages = [`Using app proxy URL: ${config.app_proxy?.url}`]
43-
if (eventType === EventType.Updated) {
44-
messages.push(`Note: Changes to app proxy prefix and subpath won't affect existing installations.`)
45-
}
46-
return messages
47-
},
4843
})
4944

5045
export default appProxySpec

‎packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.test.ts‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ describe('DevSessionLogger', () => {
172172

173173
test('logs messages', async () => {
174174
const mockExtension = {
175-
getDevSessionUpdateMessage: vi.fn().mockResolvedValue('This has been updated.'),
175+
getDevSessionUpdateMessages: vi.fn().mockResolvedValue(['This has been updated.']),
176176
entrySourceFilePath: '',
177177
devUUID: '',
178178
localIdentifier: '',
@@ -198,6 +198,32 @@ describe('DevSessionLogger', () => {
198198
]
199199
`)
200200
})
201+
202+
test('does not log messages when extension is deleted', async () => {
203+
const mockExtension = {
204+
getDevSessionUpdateMessages: vi.fn().mockResolvedValue(['This would be logged if not deleted.']),
205+
entrySourceFilePath: '',
206+
devUUID: '',
207+
localIdentifier: '',
208+
idEnvironmentVariableName: '',
209+
} as unknown as ExtensionInstance
210+
211+
const event: AppEvent = {
212+
app: {configuration: {}} as any,
213+
extensionEvents: [
214+
{
215+
type: 'deleted' as EventType,
216+
extension: mockExtension,
217+
},
218+
],
219+
path: '',
220+
startTime: [0, 0],
221+
}
222+
223+
await logger.logExtensionUpdateMessages(event)
224+
expect(output).toMatchInlineSnapshot(`[]`)
225+
expect(mockExtension.getDevSessionUpdateMessages).not.toHaveBeenCalled()
226+
})
201227
})
202228

203229
describe('logMultipleErrors', () => {

‎packages/app/src/cli/services/dev/processes/dev-session/dev-session-logger.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {UserError} from './dev-session.js'
2-
import {AppEvent} from '../../app-events/app-event-watcher.js'
2+
import {AppEvent, EventType} from '../../app-events/app-event-watcher.js'
33
import {ExtensionInstance} from '../../../../models/extensions/extension-instance.js'
44
import {outputToken, outputContent, outputDebug} from '@shopify/cli-kit/node/output'
55
import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components'
@@ -78,7 +78,8 @@ export class DevSessionLogger {
7878
const extensionEvents = event.extensionEvents ?? []
7979
const messageArrays = await Promise.all(
8080
extensionEvents.map(async (eve) => {
81-
const messages = await eve.extension.getDevSessionUpdateMessage(eve.type)
81+
if (eve.type === EventType.Deleted) return []
82+
const messages = await eve.extension.getDevSessionUpdateMessages()
8283
return messages ?? []
8384
}),
8485
)

0 commit comments

Comments
 (0)