Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions .changeset/silly-bears-share.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@clack/prompts": minor
"@clack/core": minor
---

Add async validation support to prompts, and validation state rendering to text prompts.
22 changes: 20 additions & 2 deletions packages/core/src/prompts/prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,11 @@ export default class Prompt<TValue> {
this._setUserInput('');
}

private onKeypress(char: string | undefined, key: Key) {
private async onKeypress(char: string | undefined, key: Key) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is now async but the emitter doesn't await it, so input keeps flowing while a validation is in flight. so i feel like there are 2 problems maybe i'm overthinking u tell me:

  • press return twice fast and you get 2 concurrent validations. the first one can resolve, emit submit, close the prompt and then second resolves and tries to render/emit on a finalizad prompt. state gets tangled.
  • the user can keep typing while validating, mutating value mid-flight. then the error or submit applies to a stale value, and the rendered dimmed input won't match what was validated.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

good catch!

we may be able to just return early when we're validating, i.e. ignore all key presses during validation

if (this.state === 'validating') {
return;
}

if (this._track && key.name !== 'return') {
if (key.name && this._isActionKey(char, key)) {
this.rl?.write(null, { ctrl: true, name: 'h' });
Expand Down Expand Up @@ -238,7 +242,21 @@ export default class Prompt<TValue> {

if (key?.name === 'return' && this._shouldSubmit(char, key)) {
if (this.opts.validate) {
const problem = runValidation(this.opts.validate, this.value);
const problemResult = runValidation(this.opts.validate, this.value);
let problem: string | Error | undefined;
// Only if it is not a string or an Error, we assume
// it is a Promise and await it.
if (
problemResult !== undefined &&
typeof problemResult !== 'string' &&
!(problemResult instanceof Error)
Comment on lines +250 to +252

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
problemResult !== undefined &&
typeof problemResult !== 'string' &&
!(problemResult instanceof Error)
problemResult instanceof Promise

wouldn't be a more clean approach for the check to be set this way?

) {
this.state = 'validating';
this.render();
problem = await problemResult;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

if the async validator throws (network error, w/e), this will throw inside an un-awaited async handler. that's an unhandled promise rejection, which kills the process in node. probably it might be better to do a try/catch? so that routes the thrown error into this.error / state = 'error' instead. also maybe would be worth a test case too.

} else {
problem = problemResult;
}
if (problem) {
this.error = problem instanceof Error ? problem.message : problem;
this.state = 'error';
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { Action } from './utils/settings.js';
/**
* The state of the prompt
*/
export type ClackState = 'initial' | 'active' | 'cancel' | 'submit' | 'error';
export type ClackState = 'initial' | 'active' | 'cancel' | 'submit' | 'error' | 'validating';

/**
* Typed event emitter for clack
Expand All @@ -15,6 +15,7 @@ export interface ClackEvents<TValue> {
cancel: (value?: any) => void;
submit: (value?: any) => void;
error: (value?: any) => void;
validating: (value?: any) => void;
cursor: (key?: Action) => void;
key: (key: string | undefined, info: Key) => void;
value: (value?: TValue) => void;
Expand Down
12 changes: 5 additions & 7 deletions packages/core/src/utils/validation.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { StandardSchemaV1 } from './standard-schema.js';

type MaybePromise<T> = T | Promise<T>;

/**
* A function or [Standard Schema](https://github.com/standard-schema/standard-schema)
* that validates user input. If a custom function is given, you should return a
Expand Down Expand Up @@ -33,7 +35,7 @@ import type { StandardSchemaV1 } from './standard-schema.js';
* ```
*/
export type Validate<TValue> =
| ((value: TValue | undefined) => string | Error | undefined)
| ((value: TValue | undefined) => MaybePromise<string | Error | undefined>)
| StandardSchemaV1<TValue | undefined, unknown>;

/**
Expand All @@ -45,15 +47,11 @@ export type Validate<TValue> =
export function runValidation<TValue>(
validate: Validate<TValue>,
value: TValue | undefined
): string | Error | undefined {
): MaybePromise<string | Error | undefined> {
if ('~standard' in validate) {
const result = validate['~standard'].validate(value);
// https://standardschema.dev/schema#how-to-only-allow-synchronous-validation
// TODO: https://github.com/bombshell-dev/clack/issues/92
if (result instanceof Promise) {
throw new TypeError(
'Schema validation must be synchronous. Update `validate()` and remove any asynchronous logic.'
);
return result.then((res) => res.issues?.at(0)?.message);
}
return result.issues?.at(0)?.message;
}
Expand Down
77 changes: 77 additions & 0 deletions packages/core/test/prompts/prompt.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import { MockReadable } from '../mock-readable.js';
import { MockWritable } from '../mock-writable.js';

const waitForTick = () => new Promise((resolve) => setTimeout(resolve, 0));

describe('Prompt', () => {
let input: MockReadable;
let output: MockWritable;
Expand Down Expand Up @@ -347,4 +349,79 @@
expect(instance.error).to.equal('must be "valid" (was "invalid")');
});
});

test('validates value with async validation', async () => {
const instance = new Prompt<string>({
input,
output,
render: () => 'foo',
validate: async (value) => {
await waitForTick();
return value === 'valid' ? undefined : 'Invalid value';
},
});
instance.prompt();

instance.value = 'invalid';
input.emit('keypress', '', { name: 'return' });

expect(instance.state).to.equal('validating');
await waitForTick(); // Wait for the validation to complete
expect(instance.state).to.equal('error');
expect(instance.error).to.equal('Invalid value');
});

test('accepts valid value with async validation', async () => {
const instance = new Prompt<string>({
input,
output,
render: () => 'foo',
validate: async (value) => {
await waitForTick();
return value === 'valid' ? undefined : 'Invalid value';
},
});
instance.prompt();

instance.value = 'valid';
input.emit('keypress', '', { name: 'return' });

expect(instance.state).to.equal('validating');
await waitForTick(); // Wait for the validation to complete
expect(instance.state).to.equal('submit');
expect(instance.error).to.equal('');
});

test('ignores keypresses while async validation is pending', async () => {
const eventSpy = vi.fn();
let resolveValidation: (problem: string | undefined) => void;
const validation = new Promise<string | undefined>((resolve) => {
resolveValidation = resolve;
});
const instance = new Prompt<string>({
input,
output,
render: () => 'foo',
validate: () => validation,
});
const resultPromise = instance.prompt();

instance.value = 'valid';
input.emit('keypress', '', { name: 'return' });

expect(instance.state).to.equal('validating');

instance.on('key', eventSpy);
input.emit('keypress', 'z', { name: 'z' });
input.emit('keypress', '\x03', { name: 'c' });

expect(eventSpy).not.toHaveBeenCalled();
expect(instance.state).to.equal('validating');

resolveValidation(undefined);

Check failure on line 421 in packages/core/test/prompts/prompt.test.ts

View workflow job for this annotation

GitHub Actions / scripts / run (types)

Variable 'resolveValidation' is used before being assigned.
await resultPromise;

expect(instance.state).to.equal('submit');
expect(instance.error).to.equal('');
});
});
2 changes: 2 additions & 0 deletions packages/prompts/src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ export const symbol = (state: State) => {
return styleText('yellow', S_STEP_ERROR);
case 'submit':
return styleText('green', S_STEP_SUBMIT);
case 'validating':
return styleText('dim', S_STEP_ACTIVE);
}
};

Expand Down
7 changes: 7 additions & 0 deletions packages/prompts/src/text.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,13 @@ export const text = (opts: TextOptions) => {
const value = this.value ?? '';

switch (this.state) {
case 'validating': {
const validatePrefix = hasGuide ? `${styleText('cyan', S_BAR)} ` : '';
const validatePrefixEnd = hasGuide ? styleText('cyan', S_BAR_END) : '';
const userInputText = styleText('dim', userInput);
const validateText = styleText('dim', 'Validating...');
return `${title}${validatePrefix}${userInputText}\n${validatePrefixEnd} ${validateText}\n`;
}
case 'error': {
const errorText = this.error ? ` ${styleText('yellow', this.error)}` : '';
const errorPrefix = hasGuide ? `${styleText('yellow', S_BAR)} ` : '';
Expand Down
104 changes: 104 additions & 0 deletions packages/prompts/test/__snapshots__/text.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,58 @@ exports[`text (isCI = false) > defaultValue sets the value but does not render 1
]
`;

exports[`text (isCI = false) > displays a validating message with async validation 1`] = `
[
"<cursor.hide>",
"│
◆ foo
│ _
└
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=2>",
"<erase.line><cursor.left count=1>",
"│ x█",
"<cursor.down count=2>",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ x█
└ Validating...
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"▲ foo
│ x█
└ should be xy
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ xy█
└
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ xy█
└ Validating...
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◇ foo
│ xy",
"
",
"<cursor.show>",
]
`;

exports[`text (isCI = false) > empty string when no value and no default 1`] = `
[
"<cursor.hide>",
Expand Down Expand Up @@ -349,6 +401,58 @@ exports[`text (isCI = true) > defaultValue sets the value but does not render 1`
]
`;

exports[`text (isCI = true) > displays a validating message with async validation 1`] = `
[
"<cursor.hide>",
"│
◆ foo
│ _
└
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=2>",
"<erase.line><cursor.left count=1>",
"│ x█",
"<cursor.down count=2>",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ x█
└ Validating...
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"▲ foo
│ x█
└ should be xy
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ xy█
└
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◆ foo
│ xy█
└ Validating...
",
"<cursor.backward count=999><cursor.up count=4>",
"<cursor.down count=1>",
"<erase.down>",
"◇ foo
│ xy",
"
",
"<cursor.show>",
]
`;

exports[`text (isCI = true) > empty string when no value and no default 1`] = `
[
"<cursor.hide>",
Expand Down
23 changes: 23 additions & 0 deletions packages/prompts/test/text.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,4 +238,27 @@ describe.each(['true', 'false'])('text (isCI = %s)', (isCI) => {

expect(output.buffer).toMatchSnapshot();
});

test('displays a validating message with async validation', async () => {
const result = prompts.text({
message: 'foo',
validate: async (val) => {
await new Promise((resolve) => setTimeout(resolve, 10));
return val !== 'xy' ? 'should be xy' : undefined;
},
input,
output,
});

input.emit('keypress', 'x', { name: 'x' });
input.emit('keypress', '', { name: 'return' });
await new Promise((resolve) => setTimeout(resolve, 20));
input.emit('keypress', 'y', { name: 'y' });
input.emit('keypress', '', { name: 'return' });

const value = await result;

expect(value).toBe('xy');
expect(output.buffer).toMatchSnapshot();
});
});
Loading