[Request] Support non-string select / multiselect values

[chrissantamaria]

Is your feature request related to a problem? Please describe.
The TypeScript types for select and multiselect options enforce that value fields passed extend Readonly<string>:
https://github.com/natemoo-re/clack/blob/d366a1f6e2339b5b97d0b9f42a8bdfd86333c2ba/packages/prompts/src/index.ts#L118-L120

This can be a bit cumbersome to use with non-string values, as it requires stringifying to pass to select + parsing the result returned back.

Describe the solution you'd like
Ideally (assuming no technical blockers + context I'm missing), select and multiselect could support any type of value. (perhaps naively) it looks like this could be as simple as changing the code block above to support a looser Value generic.

Describe alternatives you've considered
Stringifying + parsing as described above, though this isn't always feasible for more complex types.

Additional context
Interestingly, from my (relatively basic) testing, this only seems to be a type error. Passing in Date values works as expected - are there other types of values which may break this?

If the fix is as simple as I'm imagining, I'd be happy to open a PR and take a stab at this! However, I'm still fairly early in my testing of this library so I may be missing some important context of why this limitation exists :)

[chrissantamaria]

Thanks for looking at this so quickly! Looking at your PR answers my original question a bit - seems like the primitive type restriction is due to values being potentially stringified (and subsequently displayed) if there's no label:
https://github.com/natemoo-re/clack/blob/c2dbd27cdcc3be10a67928334463b09869f0d0c0/packages/prompts/src/index.ts#L157

On one hand I can get behind this, as non-primitives may not stringify cleanly. However, it does prevent valid use-cases of:

• value type implementing a desired, human-readable toString method
• label being provided, meaning value is never stringified -> shown anyways

The second case in particular feels like enough of a reason to remove the primitive restriction. In the label-less case, (imo) the judgement call should be up to consumers of this lib on whether or not the value.toString() implementation is sufficient. (afaik) String(option.value) will always return some sort of string, so there shouldn't be any runtime error concerns either.

That all being said, that's just my opinion! Totally understand if you'd like to keep things as-is - happy to maintain my own fork / patch in projects. Thanks again!

[ulken]

value type implementing a desired, human-readable toString method

Why not explicitly provide that as label then?

---

IMO, more complex types could be handled in user land without much overhead. They ought to have a unique identifier to use as value.

Finding the parent object from the returned value is trivial.

[chrissantamaria]

Why not explicitly provide that as label then?

That's fair - I'd be happy to provide my own label, though the types restrict me from providing any non-primitive value regardless. For example:

const today = new Date();
const tomorrow = new Date(today);
tomorrow.setDate(tomorrow.getDate() + 1);

await select({
  message: '',
  options: [
    {
      label: `Today (${today.toLocaleDateString()})`,
      value: today, // type error
    },
    {
      label: `Tomorrow (${tomrorow.toLocaleDateString()})`,
      value: tomorrow, // type error
    }
  ],
});

Even though this behaves just fine at runtime and the date values are never stringified, the types prevent this type of use case. Stringifying the value in options + decoding the final result just to satisfy TypeScript feels like unnecessary overhead + complexity.

I could see this going either way:

• remove value type restriction altogether, allowing for the above case + label-less cases (leaving control to consumers on whether value.toString() makes sense)
• only allow non-primitive values when a label is also provided (this would likely require some TypeScript magic to support both cases)

[ulken]

That's fair - I'd be happy to provide my own label, though the types restrict me from providing any non-primitive value regardless.

Right, I see your point.

Even though this behaves just fine at runtime and the date values are never stringified, the types prevent this type of use case. Stringifying the value in options + decoding the final result just to satisfy TypeScript feels like unnecessary overhead + complexity.

Like you've pointed out yourself it's not to please TypeScript per se, but rather the underlying implementation (i.e. allowing value as a fallback in a sensical way).

For the example at hand, doing a .toISOString() and then new Date() doesn't seem like much overhead, nor being complex to me. There's no performance issues to talk about either in such small data sets.

With that being said, with regards to your suggested solutions, I'm not in favor of # 1, but see no reason not to allow # 2, if technically possible (which I believe it is). I'll explore when I find the time.

[ulken]

@chrissantamaria your example above is supported in the latest release.

[chrissantamaria]

Really appreciate you taking a look at this! It does look like non-primitive types are allowed as values (🎉), but the return type of select is unfortunately unknown. Here's a full example (available on stackblitz as well):

import { select, isCancel } from '@clack/prompts';

const run = async () => {
  const today = new Date();
  const tomorrow = new Date(today);
  tomorrow.setDate(tomorrow.getDate() + 1);

  const value = await select({
    message: '',
    options: [
      {
        label: `Today (${today.toLocaleDateString()})`,
        value: today,
      },
      {
        label: `Tomorrow (${tomorrow.toLocaleDateString()})`,
        value: tomorrow,
      },
    ],
  });
  if (isCancel(value)) {
    return;
  }

  console.log({ value }); // value inferred as unknown
};

run().catch((e) => {
  console.error(e);
  process.exit(1);
});

Haven't had time to dig into the internals yet, though I'm happy to do so!

[ulken]

Sorry for not getting back to you. That's a real bummer... why, oh why, TypeScript? Can't you be a little decent and help out, if only just a little?

I took a quick look at it but wasn't able to sort it out then. I'll try to get back to it ASAP. Maybe infer could help us out here?

Not sure what's worse, if we should revert this until properly fixed or not.

Thoughts @natemoo-re?

[chrissantamaria]

I might be missing something, but just by reducing select to use a single Value generic (removing Options) it seems like the problem goes away: chrissantamaria@d174313

I imagine there's likely some reason this generic existed, though typechecking for the build still passes. If this feels like a reasonable solution I'm happy to put up a PR!

[ulken]

Seems reasonable to me! Always good with fresh eyes...

What say you @natemoo-re?

Note: there are more components using Option which would need to be changed as well, but you're probably aware.

[chrissantamaria]

Opened #102 with all Option instances updated so we can move discussion there