Skip to content

Commit b1a437e

Browse files
authored
clean-up(ui): remove CachedPrompter (#2752)
## Problem Caching at intermediate layers in an architecture, especially UI, is generally not a good idea unless there's substantial benefit for doing so. `CachedPrompte`r is also clunky and hard to use. ## Solution Remove `CachedPrompter`. Luckily I only used it in a few spots.
1 parent a3735b9 commit b1a437e

File tree

11 files changed

+84
-214
lines changed

11 files changed

+84
-214
lines changed

src/apprunner/wizards/apprunnerCreateServiceWizard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ import { Prompter } from '../../shared/ui/prompter'
1212
import { Wizard, WizardState } from '../../shared/wizards/wizard'
1313
import { AppRunnerImageRepositoryWizard } from './imageRepositoryWizard'
1414
import { AppRunnerCodeRepositoryWizard } from './codeRepositoryWizard'
15-
import { BasicExitPrompterProvider } from '../../shared/ui/common/exitPrompter'
1615
import { GitExtension } from '../../shared/extensions/git'
1716
import { makeDeploymentButton } from './deploymentButton'
1817
import { apprunnerCreateServiceDocsUrl } from '../../shared/constants'
18+
import { createExitPrompter } from '../../shared/ui/common/exitPrompter'
1919
import { DefaultIamClient } from '../../shared/clients/iamClient'
2020
import { DefaultEcrClient } from '../../shared/clients/ecrClient'
2121
import { DefaultAppRunnerClient } from '../../shared/clients/apprunnerClient'
@@ -110,7 +110,7 @@ export class CreateAppRunnerServiceWizard extends Wizard<AppRunner.CreateService
110110
super({
111111
initState,
112112
implicitState,
113-
exitPrompterProvider: new BasicExitPrompterProvider(),
113+
exitPrompterProvider: createExitPrompter,
114114
})
115115

116116
const autoDeployButton = makeDeploymentButton()

src/apprunner/wizards/codeRepositoryWizard.ts

Lines changed: 31 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,11 @@ import { createCommonButtons, createRefreshButton, QuickInputToggleButton } from
99
import { Remote } from '../../../types/git.d'
1010
import { GitExtension } from '../../shared/extensions/git'
1111
import * as vscode from 'vscode'
12-
import { CachedFunction, CachedPrompter } from '../../shared/ui/prompter'
1312
import { WizardForm } from '../../shared/wizards/wizardForm'
1413
import { createVariablesPrompter } from '../../shared/ui/common/variablesPrompter'
1514
import { AppRunnerClient } from '../../shared/clients/apprunnerClient'
1615
import { makeDeploymentButton } from './deploymentButton'
17-
import { ConnectionSummary } from 'aws-sdk/clients/apprunner'
18-
import {
19-
createLabelQuickPick,
20-
createQuickPick,
21-
DataQuickPickItem,
22-
QuickPickPrompter,
23-
} from '../../shared/ui/pickerPrompter'
16+
import { createLabelQuickPick, createQuickPick, QuickPickPrompter } from '../../shared/ui/pickerPrompter'
2417
import { createInputBox, InputBoxPrompter } from '../../shared/ui/inputPrompter'
2518
import {
2619
apprunnerConnectionHelpUrl,
@@ -29,7 +22,6 @@ import {
2922
apprunnerCreateServiceDocsUrl,
3023
} from '../../shared/constants'
3124
import { Wizard, WIZARD_BACK } from '../../shared/wizards/wizard'
32-
import { getLogger } from '../../shared/logger/logger'
3325

3426
const localize = nls.loadMessageBundle()
3527

@@ -151,71 +143,41 @@ function createPortPrompter(): InputBoxPrompter {
151143
})
152144
}
153145

154-
export class ConnectionPrompter extends CachedPrompter<ConnectionSummary> {
155-
public constructor(private readonly client: AppRunnerClient) {
156-
super()
146+
export function createConnectionPrompter(client: AppRunnerClient) {
147+
const noItemsFoundItem = {
148+
label: 'No connections found',
149+
detail: 'Click for documentation on creating a new GitHub connection for App Runner',
150+
data: {} as any,
151+
invalidSelection: true,
152+
onClick: () => vscode.env.openExternal(vscode.Uri.parse(apprunnerConnectionHelpUrl)),
157153
}
158154

159-
protected load(): Promise<DataQuickPickItem<ConnectionSummary>[]> {
160-
return this.client
161-
.listConnections({})
162-
.then(resp => {
163-
const connections = resp.ConnectionSummaryList.filter(conn => conn.Status === 'AVAILABLE').map(
164-
conn => ({
165-
label: conn.ConnectionName!,
166-
data: conn,
167-
})
168-
)
169-
170-
if (connections.length === 0) {
171-
return [
172-
{
173-
label: 'No connections found',
174-
detail: 'Click for documentation on creating a new GitHub connection for App Runner',
175-
data: {} as any,
176-
invalidSelection: true,
177-
onClick: vscode.env.openExternal.bind(
178-
vscode.env,
179-
vscode.Uri.parse(apprunnerConnectionHelpUrl)
180-
),
181-
},
182-
]
183-
} else {
184-
return connections
185-
}
186-
})
187-
.catch(err => {
188-
getLogger().error(`Failed to list GitHub connections: %O`, err)
189-
return [
190-
{
191-
label: localize(
192-
'AWS.apprunner.createService.selectConnection.failed',
193-
'Failed to list GitHub connections'
194-
),
195-
description: localize('AWS.generic.goBack', 'Click to go back'),
196-
data: WIZARD_BACK,
197-
},
198-
]
199-
})
155+
const errorItem = {
156+
label: localize('AWS.apprunner.createService.selectConnection.failed', 'Failed to list GitHub connections'),
157+
description: localize('AWS.generic.goBack', 'Click to go back'),
158+
data: WIZARD_BACK,
200159
}
201160

202-
protected createPrompter(loader: CachedFunction<ConnectionPrompter['load']>): QuickPickPrompter<ConnectionSummary> {
203-
const connections = loader()
204-
const refreshButton = createRefreshButton()
205-
const prompter = createQuickPick(connections, {
206-
title: localize('AWS.apprunner.createService.selectConnection.title', 'Select a connection'),
207-
buttons: [refreshButton, ...createCommonButtons(apprunnerConnectionHelpUrl)],
208-
})
161+
const getItems = async () => {
162+
const resp = await client.listConnections()
209163

210-
const refresh = () => {
211-
loader.clearCache()
212-
prompter.clearAndLoadItems(loader())
213-
}
164+
return resp.ConnectionSummaryList.filter(conn => conn.Status === 'AVAILABLE').map(conn => ({
165+
label: conn.ConnectionName!,
166+
data: conn,
167+
}))
168+
}
214169

215-
refreshButton.onClick = refresh
170+
const refreshButton = createRefreshButton()
171+
refreshButton.onClick = () => void prompter.clearAndLoadItems(getItems())
216172

217-
return prompter
218-
}
173+
const prompter = createQuickPick(getItems(), {
174+
errorItem,
175+
noItemsFoundItem,
176+
buttons: [refreshButton, ...createCommonButtons(apprunnerConnectionHelpUrl)],
177+
title: localize('AWS.apprunner.createService.selectConnection.title', 'Select a connection'),
178+
})
179+
180+
return prompter
219181
}
220182

221183
function createSourcePrompter(): QuickPickPrompter<AppRunner.ConfigurationSource> {
@@ -276,10 +238,9 @@ export class AppRunnerCodeRepositoryWizard extends Wizard<AppRunner.SourceConfig
276238
) {
277239
super()
278240
const form = this.form
279-
const connectionPrompter = new ConnectionPrompter(client)
280241

281-
form.AuthenticationConfiguration.ConnectionArn.bindPrompter(
282-
connectionPrompter.transform(conn => conn.ConnectionArn!)
242+
form.AuthenticationConfiguration.ConnectionArn.bindPrompter(() =>
243+
createConnectionPrompter(client).transform(conn => conn.ConnectionArn!)
283244
)
284245
form.CodeRepository.applyBoundForm(createCodeRepositorySubForm(git))
285246
form.AutoDeploymentsEnabled.setDefault(() => autoDeployButton.state === 'on')

src/apprunner/wizards/imageRepositoryWizard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ import { makeDeploymentButton } from './deploymentButton'
2121
import { IamClient } from '../../shared/clients/iamClient'
2222
import { createRolePrompter } from '../../shared/ui/common/roles'
2323
import { getLogger } from '../../shared/logger/logger'
24-
import { BasicExitPrompterProvider } from '../../shared/ui/common/exitPrompter'
2524
import { isCloud9 } from '../../shared/extensionUtilities'
2625
import { apprunnerCreateServiceDocsUrl } from '../../shared/constants'
26+
import { createExitPrompter } from '../../shared/ui/common/exitPrompter'
2727

2828
const localize = nls.loadMessageBundle()
2929

@@ -228,7 +228,7 @@ function createImageRepositorySubForm(
228228
// note: this is intentionally initialized only once to preserve caches
229229
const imageIdentifierWizard = new Wizard({
230230
initForm: new ImageIdentifierForm(ecrClient),
231-
exitPrompterProvider: new BasicExitPrompterProvider(),
231+
exitPrompterProvider: createExitPrompter,
232232
})
233233

234234
form.ImageIdentifier.bindPrompter(() =>

src/lambda/wizards/samInitWizard.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ import { createLabelQuickPick, createQuickPick, QuickPickPrompter } from '../../
3939
import { createRegionPrompter } from '../../shared/ui/common/region'
4040
import { Region } from '../../shared/regions/endpoints'
4141
import { createCommonButtons } from '../../shared/ui/buttons'
42-
import { BasicExitPrompterProvider } from '../../shared/ui/common/exitPrompter'
42+
import { createExitPrompter } from '../../shared/ui/common/exitPrompter'
4343

4444
const localize = nls.loadMessageBundle()
4545

@@ -202,7 +202,7 @@ export class CreateNewSamAppWizard extends Wizard<CreateNewSamAppWizardForm> {
202202
credentials?: AWS.Credentials
203203
}) {
204204
super({
205-
exitPrompterProvider: new BasicExitPrompterProvider(),
205+
exitPrompterProvider: createExitPrompter,
206206
})
207207

208208
this.form.runtimeAndPackage.bindPrompter(() => createRuntimePrompter(context.samCliVersion))

src/shared/clients/apprunnerClient.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export class DefaultAppRunnerClient {
4040
}
4141

4242
public async listConnections(
43-
request: AppRunner.ListConnectionsRequest
43+
request: AppRunner.ListConnectionsRequest = {}
4444
): Promise<AppRunner.ListConnectionsResponse> {
4545
return (await this.createSdkClient()).listConnections(request).promise()
4646
}

src/shared/ui/common/exitPrompter.ts

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { localize } from '../../utilities/vsCodeUtils'
77
import { StepEstimator } from '../../wizards/wizard'
88
import { createQuickPick } from '../pickerPrompter'
9-
import { Prompter, CachedPrompter, PromptResult } from '../prompter'
9+
import { Prompter, PromptResult } from '../prompter'
1010

1111
class BasicExitPrompter extends Prompter<boolean> {
1212
private _isStart = true
@@ -40,11 +40,6 @@ class BasicExitPrompter extends Prompter<boolean> {
4040
public setStepEstimator(estimator: StepEstimator<boolean>): void {}
4141
}
4242

43-
export class BasicExitPrompterProvider extends CachedPrompter<boolean, any> {
44-
protected load(...args: any) {
45-
return undefined
46-
}
47-
protected createPrompter(loader: any, state: any): Prompter<boolean> {
48-
return new BasicExitPrompter()
49-
}
43+
export function createExitPrompter() {
44+
return new BasicExitPrompter()
5045
}

src/shared/ui/inputPrompter.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
import * as vscode from 'vscode'
77
import { assign } from '../utilities/collectionUtils'
8-
import { StepEstimator, WIZARD_BACK, WIZARD_EXIT } from '../wizards/wizard'
8+
import { isValidResponse, StepEstimator, WIZARD_BACK, WIZARD_EXIT } from '../wizards/wizard'
99
import { QuickInputButton, PrompterButtons } from './buttons'
1010
import { Prompter, PromptResult } from './prompter'
1111

@@ -46,6 +46,13 @@ export function createInputBox(options?: ExtendedInputBoxOptions): InputBoxPromp
4646
return prompter
4747
}
4848

49+
export async function showInputBox(options?: ExtendedInputBoxOptions): Promise<string | undefined> {
50+
const prompter = createInputBox(options)
51+
const response = await prompter.prompt()
52+
53+
return isValidResponse(response) ? response : undefined
54+
}
55+
4956
/**
5057
* UI element that accepts user-inputted text. Wraps around {@link vscode.InputBox InputBox}.
5158
*

src/shared/ui/pickerPrompter.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as vscode from 'vscode'
77
import * as nls from 'vscode-nls'
88

9-
import { StepEstimator, WIZARD_BACK, WIZARD_EXIT } from '../wizards/wizard'
9+
import { isValidResponse, StepEstimator, WIZARD_BACK, WIZARD_EXIT } from '../wizards/wizard'
1010
import { QuickInputButton, PrompterButtons } from './buttons'
1111
import { Prompter, PromptResult, Transform } from './prompter'
1212
import { assign, isAsyncIterable } from '../utilities/collectionUtils'
@@ -151,6 +151,16 @@ export function createQuickPick<T>(
151151
return prompter
152152
}
153153

154+
export async function showQuickPick<T>(
155+
items: ItemLoadTypes<T>,
156+
options?: ExtendedQuickPickOptions<T>
157+
): Promise<T | undefined> {
158+
const prompter = createQuickPick(items, options)
159+
const response = await prompter.prompt()
160+
161+
return isValidResponse(response) ? response : undefined
162+
}
163+
154164
// Note: the generic type used in `createLabelQuickPick` is needed to infer the correct type when using string
155165
// literal types. Otherwise the narrowness of the type would be lost.
156166
/** Creates a QuickPick from normal QuickPickItems, using the `label` as the return value. */

src/shared/ui/prompter.ts

Lines changed: 1 addition & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6-
import * as _ from 'lodash'
7-
import { isValidResponse, StateWithCache, StepEstimator, WizardControl } from '../wizards/wizard'
6+
import { isValidResponse, StepEstimator, WizardControl } from '../wizards/wizard'
87

98
export type QuickPickDataType<T> = T | WizardControl | undefined
109

@@ -75,103 +74,3 @@ export abstract class Prompter<T> {
7574
protected abstract promptUser(): Promise<PromptResult<T>>
7675
public abstract setSteps(current: number, total: number): void
7776
}
78-
79-
export interface CachedPrompter<T, TState extends Record<string, unknown>> {
80-
(state: StateWithCache<TState, T>): Prompter<T>
81-
}
82-
83-
type Cacheable = (...args: (string | number | boolean | undefined)[]) => NonNullable<any>
84-
85-
/**
86-
* A wrapped function that can cache both synchronous and asynchronous return types.
87-
*
88-
* Pending promises are returned as-is, while resolved promises are 'unboxed' into their promised type.
89-
*/
90-
export interface CachedFunction<F extends (...args: any) => any> {
91-
(...args: Parameters<F>): Awaited<ReturnType<F>> | ReturnType<F>
92-
clearCache(): void
93-
supplantLast(result: Awaited<ReturnType<F>>): void
94-
}
95-
96-
// TODO: this currently just caches primitive arguments which is not very flexible
97-
// there is a 'cache-object' library that could make sense here. we could just cache the entire
98-
// argument list itself and not worry about types
99-
function createCachedFunction<F extends Cacheable>(
100-
loader: F,
101-
cache: { [key: string]: ReturnType<F> } = {},
102-
keys: Set<string> = new Set()
103-
): CachedFunction<F> {
104-
const wrapped = (...args: Parameters<F>) => {
105-
const key =
106-
args
107-
.map(arg => (arg ?? '').toString())
108-
.map(arg => (arg === '' ? arg : `${arg}${arg.length}`))
109-
.join() + '0' // Cannot index with an empty string
110-
111-
if (cache[key] !== undefined) {
112-
return cache[key]
113-
}
114-
115-
const resolved = loader(...args)
116-
cache[key] = resolved as any
117-
keys.add(key)
118-
119-
if (resolved instanceof Promise) {
120-
return resolved.then(result => {
121-
cache[key] = result
122-
return result
123-
})
124-
}
125-
126-
return resolved
127-
}
128-
129-
const clearCache = () => {
130-
keys.forEach(key => delete cache[key])
131-
keys.clear()
132-
}
133-
134-
const supplantLast = (result: ReturnType<F>) => {
135-
if (keys.size === 0) {
136-
throw new Error('Cannot evict an empty cache')
137-
}
138-
cache[[...keys.values()].pop()!] = result
139-
}
140-
141-
return Object.assign(wrapped, { clearCache, supplantLast })
142-
}
143-
144-
// Rebinds the Function function body to instead call the extended class method
145-
const EXECUTE_CALLEE = 'return arguments.callee.call.apply(arguments.callee, arguments)'
146-
/**
147-
* Convenience class that lightly wraps the creation of a Prompter with the loading of whatever resources
148-
* are required. The 'load' call is wrapped with a cache for automatic caching of its results.
149-
*/
150-
export abstract class CachedPrompter<
151-
T,
152-
TState extends Record<string, unknown> = Record<string, unknown>
153-
> extends Function {
154-
private usedKeys = new Set<string>()
155-
protected transforms: Transform<T, any>[] = []
156-
157-
public constructor() {
158-
super(EXECUTE_CALLEE)
159-
}
160-
161-
public transform<R>(callback: Transform<T, R>): CachedPrompter<R, TState> {
162-
this.transforms.push(callback)
163-
return this as unknown as CachedPrompter<R, TState>
164-
}
165-
166-
public call(state: StateWithCache<TState, T>): Prompter<T> {
167-
const cachedLoad = createCachedFunction(this.load.bind(this), state.stepCache, this.usedKeys)
168-
const prompter = this.createPrompter(cachedLoad, state)
169-
170-
this.transforms.map(prompter.transform.bind(prompter))
171-
172-
return prompter
173-
}
174-
175-
protected abstract load(...args: any): any
176-
protected abstract createPrompter(loader: CachedFunction<any>, state: TState): Prompter<T>
177-
}

0 commit comments

Comments
 (0)