clack
clack copied to clipboard
fix(@clack/core,@clack/prompts): add non empty opts type interface
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.
⚠️ No Changeset found
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.
This PR includes no changesets
When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types
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:
- it looks like it doesn't make sense to have an empty list of options for a given group (and it currenlty is buggy, see https://stackblitz.com/edit/node-51onk7?file=index.ts, the
test3group 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
optionsitself is empty (see https://stackblitz.com/edit/node-dhmjec?file=index.ts, press space to make it crash):
I tried an approach where I use aTypeError: 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)NonEmptyObjecttype onoptionsinGroupMultiSelectOptions, 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
ValueforgroupMultiselect(it ends up always beingunknown) and I didn't manage to make it work (might need to refactor to have the type ofvalueas 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.