Open kevduc opened 10 months ago
Latest commit: f663d0f7e2a37ba7a088ac08f92c9e13a72dffe7
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
Example of what the type error could look like in VSCode:
Hey @kevduc ! Thanks for this PR! 🙏🏼
I recently merged a PR to simplify types on @clack/prompts
if you want to resolve issues -- I can then be able to merge this!
I could also see this being used also on https://github.com/natemoo-re/clack/blob/7c7fde8dcdfa1edc3feb131331290affbaa6d934/packages/prompts/src/index.ts#L307 & https://github.com/natemoo-re/clack/blob/7c7fde8dcdfa1edc3feb131331290affbaa6d934/packages/prompts/src/index.ts#L423
@cpreston321 Thanks for the suggestions!
The fix for MultiSelectOptions
is pretty much the same as for select (https://github.com/natemoo-re/clack/pull/145/commits/20cde30e5e48fbe2b15a8d5dba2ae8f9845a4489).
I also had a look at GroupMultiSelectPrompt
:
test3
group looks selected, it can't be toggled, and when pressing enter it says nothing is selected), so the typing is fixed in https://github.com/natemoo-re/clack/pull/145/commits/f663d0f7e2a37ba7a088ac08f92c9e13a72dffe7:
it also looks like there's another bug when options
itself is empty (see https://stackblitz.com/edit/node-dhmjec?file=index.ts, press space to make it crash):
TypeError: Cannot read properties of undefined (reading 'group')
at CD.toggleValue (file:///home/projects/node-dhmjec/node_modules/@clack/core/dist/index.mjs:40:1272)
at Object.eval [as cb] (file:///home/projects/node-dhmjec/node_modules/@clack/core/dist/index.mjs:40:1049)
at CD.emit (file:///home/projects/node-dhmjec/node_modules/@clack/core/dist/index.mjs:35:1851)
at CD.onKeypress (file:///home/projects/node-dhmjec/node_modules/@clack/core/dist/index.mjs:35:2130)
I tried an approach where I use a NonEmptyObject
type on options
in GroupMultiSelectOptions
, i.e. something like this:
// packages\core\src\utility-types.ts
export type NonEmptyObject<T> = keyof T extends never ? never : T;
// packages\core\src\prompts\group-multiselect.ts
interface GroupMultiSelectOptions<
T extends { value: any },
U extends Record<string, NonEmptyArray<T>> = Record<string, NonEmptyArray<T>>
> extends PromptOptions<GroupMultiSelectPrompt<T, U>> {
options: NonEmptyObject<U>;
initialValues?: T['value'][];
required?: boolean;
cursorAt?: T['value'];
}
but that pretty much breaks the inference of
Value
for groupMultiselect
(it ends up always being unknown
) and I didn't manage to make it work (might need to refactor to have the type of value
as a generic instead of the type of option { value: any }
), so I've not added it to this MR, and if there's no straightforward fix, that's something that can be looked at in a different MR. An idea would be to at least do a runtime check and throw a more meaningful error.
This PR addresses this issue https://github.com/natemoo-re/clack/issues/144.
The current implementation is erroring out at runtime, the implementation in this PR shows a type error in the IDE/tsc instead.