buildo / metarpheus-io-ts

Generate domain models and client interpreting metarpheus output
MIT License
7 stars 1 forks source link

Do not generate Iso instances for newtypes #82

Open gabro opened 5 years ago

gabro commented 5 years ago

We currently generate Iso instances for each newtype.

As a refresher, Iso proves an isomorphism exists between two types, but this is not always true in our apps.

For example, take NonBlankString: using iso<NonBlankString>().wrap("") is unsafe, because it produces a invalid NonBlankString.

For NonBlankString we're better served by a Prism, which provides a getOption operation which returns an Option<NonBlankString>:

const nonBlankStringPrism = prism<NonBlankString>(s => s.trim().length > 0)
nonBlankStringPrism.getOption("a") // Some(a)
nonBlankStringPrism.getOption("")  // None

I propose we don't generate Iso instances for newtypes, and delegate this to the app's developers.

fes300 commented 5 years ago

I agree about stripping the Iso generation, I'm not 100% about using prism just to get an option (why not using fromPredicate?) but I am not an expert about optics, maybe there is some goodness I am not aware of 🙃

gabro commented 5 years ago

fromPredicate for what? Can you make an example?

fes300 commented 5 years ago

I was looking at this implementation of prism but looking closer your example seems to have a slightly different (and more convenient) signature.

the fromPredicate I was thinking of is the one from fp-ts/Option

used like this:

fromPredicate(isNonEmptyString)("a")
gabro commented 5 years ago

the interesting thing about definiting an instance of Prism is that you gain a few interesting capabilities as well. For example, you can go the other direction using reverseGet:

const nonBlankStringPrism = prism<NonBlankString>(s => s.trim().length > 0)
declare const someString: NonBlankString;

nonBlankStringPrism.reverseGet(someString) // string

or another example:

declare function doSomething(nbs: NonBlankString): NonBlankString;
nonBlankStringPrism.modifyOption(doSomething)("foo") // Some(doSomething("foo"))
nonBlankStringPrism.modifyOption(doSomething)("")    // None
fes300 commented 5 years ago

that's exactly the kind of goodies I was talking about :) although my biggest doubt about using prisms (be aware that I know very little about the topic as I never used them) is their use outside the "Optics" scope.

What I mean is that the examples you gave can be rewritten using constructs we already are familiar with (they are a little less flexible but I can't see many use cases where prism syntax would be a plus):

const optionNonEmptyString = fromPredicate(s => s.trim().length > 0)

optionNonEmptyString("") // None
optionNonEmptyString("").getOrElse("") // Reverse
optionNonEmptyString("foo").map(doSomething) // Some(doSomething("foo"))

etc...

I always tought about prisms lenses etc.. as a great tool because of how they compose so it seems strange to introduce them in our codebase without mentioning optics