Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/src/dev/activation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,7 @@ async function openStorageFromInput() {
title: 'Enter a key',
})
} else if (target === 'globalsView') {
return new SkipPrompter('')
return new SkipPrompter()
} else if (target === 'globals') {
// List all globalState keys in the quickpick menu.
const items = globalState
Expand Down Expand Up @@ -483,7 +483,7 @@ async function resetState() {

this.form.key.bindPrompter(({ target }) => {
if (target && resettableFeatures.some((f) => f.name === target)) {
return new SkipPrompter('')
return new SkipPrompter()
}
throw new Error('invalid feature target')
})
Expand Down
15 changes: 4 additions & 11 deletions packages/core/src/shared/ui/common/skipPrompter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,17 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { StepEstimator } from '../../wizards/wizard'
import { StepEstimator, WIZARD_SKIP } from '../../wizards/wizard'
import { Prompter, PromptResult } from '../prompter'

/** Pseudo-prompter that immediately returns a value (and thus "skips" a step). */
/** Prompter that return SKIP control signal to parent wizard */
export class SkipPrompter<T> extends Prompter<T> {
/**
* @param val Value immediately returned by the prompter.
*/
constructor(public readonly val: T) {
Copy link
Member

@roger-zhangg roger-zhangg Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous implementation supports resolving the value val it was assigned in the beginning. but this is removed in the new solution. Is this intended?

Copy link
Contributor Author

@vicheey vicheey Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Since we have SKIP signal. Resolving value was a workaround.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would there be a case we actually want it to resolve some default value?

Copy link
Member

@roger-zhangg roger-zhangg Dec 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A straight forward case would be in bindPrompter, we have multiple if/else based on previous states, and maybe there are two case leads to skip prompter, but we want different value for this field even though it is skipped.

With previous solution we will be able to do as the follows

        form.stuff.bindPrompter(
            (f) => {
                if (f===1) {
                     return skipPrompter(1)
                }
                if (f===2) {
                     return skipPrompter(2)
                }
               else{
               return createQuickPick
              }
            },
            { setDefault: "skipped"} // set default won't be ideal in this case
        )

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous workaround for skipping prompter was to automatically resolve the value and skip displaying UI to customer. However, that did not work for backward direction since the auto resolve always revert the backward flow to forward flow.

So we are splitting the skipping case from setting default value case. For how to use the SkipPrompter, please follow this example: aws-toolkit-vscode/packages/core/src/dev/activation.ts at master · vicheey/aws-toolkit-vscode

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can consider resolving the value when forward and skip when backwards right?

constructor() {
super()
}

protected async promptUser(): Promise<PromptResult<T>> {
const promptPromise = new Promise<PromptResult<T>>((resolve) => {
resolve(this.val)
})

return await promptPromise
return WIZARD_SKIP
}

public get recentItem(): any {
Expand Down
63 changes: 63 additions & 0 deletions packages/core/src/shared/ui/nestedWizardPrompter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
/*!
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0
*/

import { Wizard, WizardOptions } from '../wizards/wizard'
import { Prompter } from './prompter'
import { WizardPrompter } from './wizardPrompter'
import { createHash } from 'crypto'

/**
* An abstract class that extends the base Wizard class plus the ability to
* use other wizard classes as prompters
*/
export abstract class NestedWizard<T> extends Wizard<T> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name seems confusing, unless I'm mistaken. This class is a Wizard that contains nested wizards, it's not itself nested. Right? So maybe a better name is Wizards (plural). That also improves discoverability because it sorts alphabetically next to Wizard.

/**
* Map to store memoized wizard instances using SHA-256 hashed keys
*/
private wizardInstances: Map<string, any> = new Map()

public constructor(options?: WizardOptions<T>) {
super(options)
}

/**
* Creates a prompter for a wizard instance with memoization.
*
* @template TWizard - The type of wizard, must extend Wizard<TState>
* @template TState - The type of state managed by the wizard
*
* @param wizardClass - The wizard class constructor
* @param args - Constructor arguments for the wizard instance
*
* @returns A wizard prompter to be used as prompter
*
* @example
* // Create a prompter for SyncWizard
* const prompter = this.createWizardPrompter<SyncWizard, SyncParams>(
* SyncWizard,
* template.uri,
* syncUrl
* )
*
* @remarks
* - Instances are memoized using a SHA-256 hash of the wizard class name and arguments
* - The same wizard instance is reused for identical constructor parameters for restoring wizard prompter
* states during back button click event
*/
protected createWizardPrompter<TWizard extends Wizard<TState>, TState>(
wizardClass: new (...args: any[]) => TWizard,
...args: ConstructorParameters<new (...args: any[]) => TWizard>
): Prompter<TState> {
const memoizeKey = createHash('sha256')
.update(wizardClass.name + JSON.stringify(args))
.digest('hex')

if (!this.wizardInstances.get(memoizeKey)) {
this.wizardInstances.set(memoizeKey, new wizardClass(...args))
}

return new WizardPrompter(this.wizardInstances.get(memoizeKey)) as Prompter<TState>
}
}
25 changes: 22 additions & 3 deletions packages/core/src/shared/ui/wizardPrompter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,29 @@
* SPDX-License-Identifier: Apache-2.0
*/

import { StepEstimator, Wizard } from '../wizards/wizard'
import _ from 'lodash'
import { StepEstimator, Wizard, WIZARD_BACK, WIZARD_SKIP } from '../wizards/wizard'
import { Prompter, PromptResult } from './prompter'

/**
* Wraps {@link Wizard} object into its own {@link Prompter}, allowing wizards to use other
* wizards in their flows.
*
* @remarks
* - This class should only be used inside wizard classes that extend {@link NestedWizard}.
* - See examples:
* - {@link SingleNestedWizard}
* - {@link DoubleNestedWizard}
*/

// eslint-disable-next-line @typescript-eslint/naming-convention
export const WIZARD_PROMPTER = 'WIZARD_PROMPTER'

export class WizardPrompter<T> extends Prompter<T> {
public get recentItem(): any {
return undefined
}
public set recentItem(response: any) {}

private stepOffset: number = 0
private response: T | undefined

Expand Down Expand Up @@ -51,6 +61,15 @@ export class WizardPrompter<T> extends Prompter<T> {

protected async promptUser(): Promise<PromptResult<T>> {
this.response = await this.wizard.run()
return this.response

if (this.response === undefined) {
return WIZARD_BACK as PromptResult<T>
} else if (_.isEmpty(this.response)) {
return WIZARD_SKIP as PromptResult<T>
}

return {
...this.response,
} as PromptResult<T>
}
}
21 changes: 20 additions & 1 deletion packages/core/src/shared/wizards/stateController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,16 @@ export enum ControlSignal {
Retry,
Exit,
Back,
Continue,
Skip,
}

/**
* Value for indicating current direction of the wizard
* Created mainly to support skipping prompters
*/
export enum DIRECTION {
Forward,
Backward,
}

export interface StepResult<TState> {
Expand Down Expand Up @@ -39,9 +48,11 @@ export class StateMachineController<TState> {
private extraSteps = new Map<number, Branch<TState>>()
private steps: Branch<TState> = []
private internalStep: number = 0
private direction: DIRECTION

public constructor(private state: TState = {} as TState) {
this.previousStates = [_.cloneDeep(state)]
this.direction = DIRECTION.Forward
}

public addStep(step: StepFunction<TState>): void {
Expand Down Expand Up @@ -71,12 +82,14 @@ export class StateMachineController<TState> {

this.state = this.previousStates.pop()!
this.internalStep -= 1
this.direction = DIRECTION.Backward
}

protected advanceState(nextState: TState): void {
this.previousStates.push(this.state)
this.state = nextState
this.internalStep += 1
this.direction = DIRECTION.Forward
}

protected detectCycle(step: StepFunction<TState>): TState | undefined {
Expand Down Expand Up @@ -105,6 +118,12 @@ export class StateMachineController<TState> {
}

if (isMachineResult(result)) {
if (result.controlSignal === ControlSignal.Skip) {
/**
* Depending on current wizard direction, skip signal get converted to forward or backward control signal
*/
result.controlSignal = this.direction === DIRECTION.Forward ? undefined : ControlSignal.Back
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would this work on Wizard prompter? Will it able to identify the direction of it's parent wizard?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The WizardPrompter is still considered a prompter. It is not responsible for identifying parent wizard direction.

This state management functionality belongs to the stateController object within the Wizard class. Each Wizard is responsible for prompting, receiving, and processing answers from prompters (including WizardPrompter). Each answer contains data and a ControlSignal (undefined indicates no direction change). The Wizard class's stateController tracks the current direction and uses the ControlSignal from the prompter to manage direction changes.

How do we restore the state of WizardPrompter when a back button is clicked?
Well, we do not restore the state of the WizardPrompter object, we always instantiate a new instance of this class. We, however, restore the state of Wizard class used for instantiate the WizardPrompter. Again, the (parent) wizard class is responsible for this (reference)

}
return result
} else {
return { nextState: result }
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/shared/wizards/wizard.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,12 @@ export const WIZARD_RETRY = {
// eslint-disable-next-line @typescript-eslint/naming-convention
export const WIZARD_BACK = { id: WIZARD_CONTROL, type: ControlSignal.Back, toString: () => makeControlString('Back') }
// eslint-disable-next-line @typescript-eslint/naming-convention
export const WIZARD_SKIP = { id: WIZARD_CONTROL, type: ControlSignal.Skip, toString: () => makeControlString('Skip') }
// eslint-disable-next-line @typescript-eslint/naming-convention
export const WIZARD_EXIT = { id: WIZARD_CONTROL, type: ControlSignal.Exit, toString: () => makeControlString('Exit') }

/** Control signals allow for alterations of the normal wizard flow */
export type WizardControl = typeof WIZARD_RETRY | typeof WIZARD_BACK | typeof WIZARD_EXIT
export type WizardControl = typeof WIZARD_RETRY | typeof WIZARD_BACK | typeof WIZARD_EXIT | typeof WIZARD_SKIP

export function isWizardControl(obj: any): obj is WizardControl {
return obj !== undefined && obj.id === WIZARD_CONTROL
Expand Down Expand Up @@ -269,9 +271,7 @@ export class Wizard<TState extends Partial<Record<keyof TState, unknown>>> {

if (isValidResponse(answer)) {
state.stepCache.picked = prompter.recentItem
}

if (!isValidResponse(answer)) {
} else {
delete state.stepCache.stepOffset
}

Expand Down
22 changes: 18 additions & 4 deletions packages/core/src/shared/wizards/wizardForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ interface ContextOptions<TState, TProp> {
* in a single resolution step then they will be added in the order in which they were
* bound.
*/
showWhen?: (state: WizardState<TState>) => boolean
showWhen?: (state: WizardState<TState>) => boolean | Promise<boolean>
/**
* Sets a default value to the target property. This default is applied to the current state
* as long as the property has not been set.
Expand Down Expand Up @@ -135,16 +135,30 @@ export class WizardForm<TState extends Partial<Record<keyof TState, unknown>>> {
this.formData.set(key, { ...this.formData.get(key), ...element })
}

public canShowProperty(prop: string, state: TState, defaultState: TState = this.applyDefaults(state)): boolean {
public canShowProperty(
prop: string,
state: TState,
defaultState: TState = this.applyDefaults(state)
): boolean | Promise<boolean> {
const current = _.get(state, prop)
const options = this.formData.get(prop) ?? {}

if (isAssigned(current) || checkParent(prop, state, options)) {
return false
}

if (options.showWhen !== undefined && !options.showWhen(defaultState as WizardState<TState>)) {
return false
if (options.showWhen !== undefined) {
const showStatus = options.showWhen(defaultState as WizardState<TState>)
if (showStatus instanceof Promise) {
return showStatus
.then((result) => {
return result
})
.catch(() => {
return false // Default to not showing if there's an error
})
}
return showStatus
}

return options.provider !== undefined
Expand Down
Loading
Loading