seasonedcc / composable-functions

Types and functions to make composition easy and safe
MIT License
636 stars 13 forks source link

Coupling with zod #78

Closed decs closed 10 months ago

decs commented 1 year ago

Hi! I'm curious on the reasoning for picking zod as the only validation lib supported. Do you feel that restricts adoption?

I faced a similar situation and ended up building an reusable universal adapter (typeschema), similar to tRPC's approach. Would something like that be useful to domain-functions? I'm open to working together to make this lib available to more devs. Thanks!

gustavoguichard commented 1 year ago

Hey @decs, thanks for reaching out. We never had interest but you're the second person to ask that. I'm so used to work with Zod I don't even think about that. My only concern is introducing complexity for the inference but as far as we could find a common ground between libraries I'm all ears.

Maybe if they follow a contract such as:

type Infer<T> = T extends {
  parse: (o: unknown) => infer O
} ? O : never

That wouldn't introduce major changes and might require just an adapter for other parsers.

What do you think?

gustavoguichard commented 1 year ago

Update: I just realized you already suggested an adapter 😅 Yeah, my concerns are still the same but there seems to have a way.

What do you think @diogob ?

diogob commented 1 year ago

I'm definitely more comfortable now that the library is more mature (compared to when we first discussed this possibility). I'm still not willing to spend much time on this. However, if we can build a version where the types remains reasonably readable, and without any test or README changes, I would be onboard.

danielweinmann commented 1 year ago

I'm very afraid of the architectural divergence that this would create. Every future improvement in the types or implementation would require us to keep the features and limitations of other parsers in mind. It would also lead our user base to expect adaptors to every new parser launched.

I think we should pick one parser and stick to it. And if we decide to change, we change the dependency in a major version instead of diverging.

decs commented 1 year ago

What would be a limitation you can foresee?

The internal usage of zod appears to be only for inferring the schema type and doing the actual validation check, which is compatible with all schema validation libs out there.

Also, if a new use case arises in the future, you could make the zod peer dependency optional and dynamically import it. Then you could still have special behaviors for people using zod.

let Zod = undefined;
try {
  Zod = await import('zod');
} catch (_e) {}

if (Zod != null) {
  // zod-specific new behavior
} else {
  // general behavior
}

And on the concern of having to keep new adapters up-to-date, that's precisely the reason I suggested using typeschema. That way, you can delegate keeping things updated to typeschema and get the new adapters for free.

decs commented 1 year ago

I can try making some time this week to show how the integration would look like and send a pull request

marcus-sa commented 1 year ago

We're using https://deepkit.io/library/type which is far superior to Zod.

diogob commented 11 months ago

Moved the development to #103

It seems to me that how our users build their parsers is not really our concern. Therefore allowing users to bring their own parser looks like a desirable feature. We just need a very straightforward interface for parsing, representing errors and a type that returns the output of a parser.

typeschema fits. It also does not add a lot of burden on our bundle (specially after ditching qs).

Since we can seamlessly upgrade, at least so far in my testing, there is no breaking change and it could be released as a minor.

The alternative of using a simpler interfaces approach would probably work on a few cases, but it can create more burden since we either would have to write some adapters or make a breaking change forcing our users to provide one.

Another point in favour is that it seems that reverting this change would be trivial and also non-breaking (but we gain a slightly more flexible code).

gustavoguichard commented 10 months ago

This issue is solved by #114 which will be released in v2.4.0

gustavoguichard commented 1 month ago

Hey guys, just an update that we now are even less coupled with zod and we built an example which uses arktype.