Improve type inference with `NoInfer` type (`check`, `forward`, ...)

[imcotton]

For example:

// Number.isSafeInteger(number: unknown): boolean

const safeInteger = v.pipe(

  v.number(),

  check2(Number.isSafeInteger),

  // Type 'unknown' is not assignable to type 'number'
  v.check(Number.isSafeInteger),

);

declare function check2<TInput>(
  requirement: (input: NoInfer<TInput>) => boolean
): v.CheckAction<TInput, undefined>;

I think a lot of places could be adopt this while working with pipe.

---

ref:

• https://www.typescriptlang.org/docs/handbook/utility-types.html#noinfertype
• https://devblogs.microsoft.com/typescript/announcing-typescript-5-4/#the-noinfer-utility-type

[fabian-hiller]

Thanks for the tip! I think it makes sense, but I need to investigate it further. So the basic idea or problem is that in this case TS is inferring the type from the argument of requirement instead of inferring it from the previous pipe element, and by using NoInfer we could tell TS to change this behavior and thus eliminate the TS error in such cases. Perhaps NoInfer could help us improve some other type declarations as well.

[imcotton]

Yes, I think overall if will improve DX from using the APIs, to adopt it throughout the codebase need more tests and measures to make sure it compatible with existing layers, that indeed takes some times to investigate over.

[EltonLobo07]

I think some of the APIs can work like NoInfer if we use a pattern like shown below:

const SafeIntegerSchema = v.pipe(
  v.string(),
  v.check(value => Number.isSafeInteger(value))
);

Obviously, this is not good (creating a function wrapper) but is a possible solution for some cases right now. I agree, the behavior described in this GitHub issue can improve DX for similar cases.

[fabian-hiller]

I definitely want to improve our types with NoInfer, but I'm not sure if we should wait a bit and reschedule it for our v2 release, because with this change we would force users to upgrade to TypeScript v5.4, and if they can't, we would prevent them from using Valibot at all.

[imcotton]

I think it's nice to have yet not urgent, thus v2 sounds good to me.

[fabian-hiller]

I am thinking about implementing NoInfer while we are in v1 beta and RC, because I think the upgrade from our current required TS version to v5.4 is seamless, and if no one complains with good reason, I will keep it for our v1 release.

[imcotton]

That's great, hope it won't adding too much burdens. Also feel free to defer it if somehow causing too much of effort given this is rather a niche feature after all.

[fabian-hiller]

My research has shown that NoInfer improves DX in your examples, but makes it worse in another.

// This works now
const Schema1 = v.pipe(v.number(), v.check(Number.isSafeInteger));

// But this code breaks because `input: number` is ignored
const isSafeInteger = v.check((input: number) => Number.isSafeInteger(input));
const Schema2 = v.pipe(v.number(), isSafeInteger);

My current conclusion is that it is probably better to keep the current implementation without NoInfer and force users to use a wrapper function, even though your example is probably more common than mine. I welcome feedback, especially on the long term. This is not a final decision.

[imcotton]

I see, even though I think 90% use cases of v.check would be within v.pipe chain, it's unfortunate that NoInfer not play along with standalone declared schema, in which I too think not worth the trade off at the moment, maybe put it on hold for now (or close).

[fabian-hiller]

I am closing this issue due to inactivity. I welcome feedback if there is still interest in this change for our v2 version.