Fix missing TS warnings in pipe method

[fabian-hiller]

Fix #669

[EltonLobo07]

I think we might not need to change this file. What do you think?

[fabian-hiller]

The same answer applies here, but feel free to give me feedback.

[EltonLobo07]

Yes, I prefer to avoid using any whenever possible. Its better to be strict wherever possible and reduce the strictness only when needed. It might not make a huge difference now but later if we decide to extend the types, having unknown might save us from undiscovered bugs. But thats just my view, its up to you.

[fabian-hiller]

I thought about it, and I think I prefer consistency over strictness in this case, but I will think about it again before merging. Do you have any other feedback? If we're both happy and can't find any major drawbacks, we should keep in mind to update the docs before merging.

[EltonLobo07]

I have no additional feedback. The only drawback is that we may lose some type safety, which was already mentioned here. Since we started working with unknown, the code won't throw unexpected errors after changing some type arguments from unknown to any but we will need to be careful when modifying the code after this PR is merged. If you are happy with the changes, have no concerns and have no additional changes to make, let me know. I can work on updating the docs unless you want to update the docs.

[fabian-hiller]

The only drawback is that we may lose some type safety

I think in both cases it doesn't matter. For InferInput, InferOutput and InferIssue it makes no difference, and for GenericMetadata using any is not really a drawback because I cannot think of a case where it makes a difference. This is because BaseMetadata does not contain a _run method that could be called with an invalid type when using any. But if we add such a method in the future, we will have to change it to any anyway to support any metadata action when GenericMetadata is used as a generic.

We could basically follow 3 paths. Maximum consistency (and therefore more any whenever it makes no difference), partial consistency (and therefore less any, this is our current solution), or full consistency (no unnecessary any).

Another thing we could consider changing is to remove the generics from GenericSchema to force users to use BaseSchema whenever they want something specific. This could lead to safer and more accurate code, as generics with defaults are often not implemented correctly because TS does not complain when they are unintentionally skipped. This change would allow us to use GenericSchema instead of BaseSchema<unknown, unknown, BaseIssue<unknown>> whenever any schema is accepted, to simplify implementation and documentation. The same goes for actions.

[fabian-hiller]

Another thing we could consider changing is to remove the generics from GenericSchema ...

Okay. This is a bad idea. Some people write code like this:

import * as v from 'valibot';

type MyType = { key: string };

const Schema = v.object({ key: v.string() }) satisfies v.GenericSchema<MyType>;

And chancing GenericSchema would break this pattern, and using BaseSchema would lead to a bad DX.

[fabian-hiller]

I will wait for your feedback on my comment yesterday and then make a decision and release a new version. Feel free to update the docs after it is clear how we will proceed.

[EltonLobo07]

I think in both cases it doesn't matter.

After your explanation and thinking about it, I agree. It makes sense.

I now think following any of the two paths (the current solution or no unnecessary any) won't make a lot of difference when it comes to type safety, so we are free to choose any one (the current solution or no unnecessary any).

However, there is one case where I would prefer no unnecessary any to support your idea:

use GenericSchema instead of BaseSchema<unknown, unknown, BaseIssue> whenever any schema is accepted, to simplify implementation and documentation.

Changing the BaseSchema type's reference function's return type to BaseSchema<unknown, unknown, BaseIssue<unknown>>. Same change for async too. We can then use GenericSchema wherever we need any schema. 

[fabian-hiller]

Thanks for your feedback and cooperation! I think we are good now and I will move on with the docs to be able to merge this PR because I want to release a new version probably today. But feel free to give me feedback if you see something.

[fabian-hiller]

v0.38.0 is available 🚀