Skip to content

Commit c9ecb60

Browse files
nickwesselmannickwesselman
authored
Merge pull request #6203 from Shopify/app-proxy-dev-logs
Improve CLI messaging for app proxy development
2 parents f9bfb32 + e0845e9 commit c9ecb60

File tree

8 files changed

+103
-29
lines changed

8 files changed

+103
-29
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -445,9 +445,9 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
445445
}
446446
}
447447

448-
async getDevSessionUpdateMessage(): Promise<string | undefined> {
449-
if (!this.specification.getDevSessionUpdateMessage) return undefined
450-
return this.specification.getDevSessionUpdateMessage(this.configuration)
448+
async getDevSessionUpdateMessages(): Promise<string[] | undefined> {
449+
if (!this.specification.getDevSessionUpdateMessages) return undefined
450+
return this.specification.getDevSessionUpdateMessages(this.configuration)
451451
}
452452

453453
/**

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
7474
buildValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
7575
hasExtensionPointTarget?(config: TConfiguration, target: string): boolean
7676
appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[]
77-
getDevSessionUpdateMessage?: (config: TConfiguration) => Promise<string>
77+
getDevSessionUpdateMessages?: (config: TConfiguration) => Promise<string[]>
7878
patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void
7979

8080
/**
@@ -186,7 +186,7 @@ export function createExtensionSpecification<TConfiguration extends BaseConfigTy
186186
reverseTransform: spec.transformRemoteToLocal,
187187
experience: spec.experience ?? 'extension',
188188
uidStrategy: spec.uidStrategy ?? (spec.experience === 'configuration' ? 'single' : 'uuid'),
189-
getDevSessionUpdateMessage: spec.getDevSessionUpdateMessage,
189+
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
190190
}
191191
const merged = {...defaults, ...spec}
192192

@@ -231,7 +231,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
231231
appModuleFeatures?: (config?: TConfiguration) => ExtensionFeature[]
232232
transformConfig: TransformationConfig | CustomTransformationConfig
233233
uidStrategy?: UidStrategy
234-
getDevSessionUpdateMessage?: (config: TConfiguration) => Promise<string>
234+
getDevSessionUpdateMessages?: (config: TConfiguration) => Promise<string[]>
235235
patchWithAppDevURLs?: (config: TConfiguration, urls: ApplicationURLs) => void
236236
}): ExtensionSpecification<TConfiguration> {
237237
const appModuleFeatures = spec.appModuleFeatures ?? (() => [])
@@ -245,7 +245,7 @@ export function createConfigExtensionSpecification<TConfiguration extends BaseCo
245245
transformRemoteToLocal: resolveReverseAppConfigTransform(spec.schema, spec.transformConfig),
246246
experience: 'configuration',
247247
uidStrategy: spec.uidStrategy ?? 'single',
248-
getDevSessionUpdateMessage: spec.getDevSessionUpdateMessage,
248+
getDevSessionUpdateMessages: spec.getDevSessionUpdateMessages,
249249
patchWithAppDevURLs: spec.patchWithAppDevURLs,
250250
})
251251
}

‎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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,15 @@ 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(',')
5353
.map((scope) => scope.trim())
5454
.join(', ')
5555
: config.access_scopes?.required_scopes?.join(', ')
5656

57-
return scopesString ? `Access scopes auto-granted: ${scopesString}` : `App has been installed`
57+
return [scopesString ? `Access scopes auto-granted: ${scopesString}` : `App has been installed`]
5858
},
5959
patchWithAppDevURLs: (config, urls) => {
6060
if (urls.redirectUrlWhitelist) {

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import spec from './app_config_app_proxy.js'
1+
import spec, {type AppProxyConfigType} from './app_config_app_proxy.js'
22
import {placeholderAppConfiguration} from '../../app/app.test-data.js'
33
import {describe, expect, test} from 'vitest'
44

@@ -50,4 +50,37 @@ describe('app_config_app_proxy', () => {
5050
})
5151
})
5252
})
53+
54+
describe('getDevSessionUpdateMessages', () => {
55+
test('should return both messages when app_proxy config is present', async () => {
56+
// Given
57+
const config: AppProxyConfigType = {
58+
app_proxy: {
59+
url: 'https://my-proxy.dev',
60+
subpath: 'apps',
61+
prefix: 'proxy',
62+
},
63+
}
64+
65+
// When
66+
const result = await spec.getDevSessionUpdateMessages!(config)
67+
68+
// Then
69+
expect(result).toEqual([
70+
'Using URL: https://my-proxy.dev',
71+
'Any changes to prefix and subpath will only apply to new installs',
72+
])
73+
})
74+
75+
test('should return empty array when no app_proxy config is present', async () => {
76+
// Given
77+
const config: AppProxyConfigType = {}
78+
79+
// When
80+
const result = await spec.getDevSessionUpdateMessages!(config)
81+
82+
// Then
83+
expect(result).toEqual([])
84+
})
85+
})
5386
})

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,8 @@ const AppProxySchema = BaseSchema.extend({
1313
.optional(),
1414
})
1515

16+
export type AppProxyConfigType = zod.infer<typeof AppProxySchema>
17+
1618
export const AppProxySpecIdentifier = 'app_proxy'
1719

1820
const AppProxyTransformConfig: TransformationConfig = {
@@ -25,6 +27,12 @@ const appProxySpec: ExtensionSpecification = createConfigExtensionSpecification(
2527
identifier: AppProxySpecIdentifier,
2628
schema: AppProxySchema,
2729
transformConfig: AppProxyTransformConfig,
30+
getDevSessionUpdateMessages: async (config) => {
31+
if (!config.app_proxy) {
32+
return []
33+
}
34+
return [`Using URL: ${config.app_proxy.url}`, `Any changes to prefix and subpath will only apply to new installs`]
35+
},
2836
patchWithAppDevURLs: (config, urls) => {
2937
if (urls.appProxy) {
3038
config.app_proxy = {

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

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,12 @@ 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: '',
179179
idEnvironmentVariableName: '',
180+
handle: 'test-extension',
180181
} as unknown as ExtensionInstance
181182

182183
const event: AppEvent = {
@@ -194,10 +195,37 @@ describe('DevSessionLogger', () => {
194195
await logger.logExtensionUpdateMessages(event)
195196
expect(output).toMatchInlineSnapshot(`
196197
[
197-
"└ This has been updated.",
198+
"[90m└ [39mThis has been updated.",
198199
]
199200
`)
200201
})
202+
203+
test('does not log messages when extension is deleted', async () => {
204+
const mockExtension = {
205+
getDevSessionUpdateMessages: vi.fn().mockResolvedValue(['This would be logged if not deleted.']),
206+
entrySourceFilePath: '',
207+
devUUID: '',
208+
localIdentifier: '',
209+
idEnvironmentVariableName: '',
210+
handle: 'test-extension',
211+
} as unknown as ExtensionInstance
212+
213+
const event: AppEvent = {
214+
app: {configuration: {}} as any,
215+
extensionEvents: [
216+
{
217+
type: 'deleted' as EventType,
218+
extension: mockExtension,
219+
},
220+
],
221+
path: '',
222+
startTime: [0, 0],
223+
}
224+
225+
await logger.logExtensionUpdateMessages(event)
226+
expect(output).toMatchInlineSnapshot(`[]`)
227+
expect(mockExtension.getDevSessionUpdateMessages).not.toHaveBeenCalled()
228+
})
201229
})
202230

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

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

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
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'
6-
import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array'
76
import {Writable} from 'stream'
87

98
export class DevSessionLogger {
@@ -77,16 +76,22 @@ export class DevSessionLogger {
7776
async logExtensionUpdateMessages(event?: AppEvent) {
7877
if (!event) return
7978
const extensionEvents = event.extensionEvents ?? []
80-
const messages = getArrayRejectingUndefined(
81-
await Promise.all(
82-
extensionEvents.map(async (eve) => {
83-
const message = await eve.extension.getDevSessionUpdateMessage()
84-
return message ? message : undefined
85-
}),
86-
),
79+
const messageArrays = await Promise.all(
80+
extensionEvents.map(async (eve) => {
81+
// Don't log messages for deleted extensions
82+
if (eve.type === EventType.Deleted) return []
83+
const messages = await eve.extension.getDevSessionUpdateMessages()
84+
return messages?.map((message) => ({message, prefix: eve.extension.handle})) ?? []
85+
}),
8786
)
87+
const messages = messageArrays.flat()
8888

89-
const logPromises = messages.map((message) => this.log(`└ ${message}`, 'app-preview'))
89+
const logPromises = messages.map((message, index) => {
90+
const messageContent = outputContent`${outputToken.gray(index === messages.length - 1 ? '└ ' : '│ ')}${
91+
message.message
92+
}`.value
93+
return this.log(messageContent, message.prefix)
94+
})
9095
await Promise.all(logPromises)
9196
}
9297

0 commit comments

Comments
 (0)