Closed teevik closed 5 years ago
I'll make a pull request if that's ok
hi, thank you for the feedback! In this case I'm not sure the idea with two generics is actually a progress. One thing is that in that concrete case with the JSX, I think this would work:
const content = results.match({
None: () => <Loader />,
Some: movies => <>{movies.map(movie => <MoviesListItem key={movie.id} {...movie} />)}</>
})
the other thing is that I think you can achieve what you want already with the current API. In the end if you want to return an or type with both options, you can just force typescript to use the or type, and it'll work already today, which means this would work:
const content = results.match<JSX.Element|JSX.Element[]>({
None: () => <Loader />,
Some: movies => movies.map(movie => <MoviesListItem key={movie.id} {...movie} />)
})
But, granted, in that case you get no inference for free.
Still, for now I think the way prelude does it for now is probably the right way. The way match
works follows a general principle in functional programming, it's a catamorphism, and that has proven useful in many contexts and libraries, for instance:
and maybe others for sure. If you check these links, they all use one generic only for the return type, so I think that's the better way. But I'm always interested if you have a concrete example or if the "workarounds" i gave are not good enough! By default though, because the concept of catamorphism is so accepted in functional programming, I probably wouldn't touch match
, but we can add another function for some special use-case, and either rename the current match
and have the new function named match
. But currently I think the current setup found in prelude is OK.
Thank you again for the feedback, and let me know what you think!
Oh yeah, I'm definitely fine with it staying like that.
I also found out you can use
results.match<React.ReactNode>({
...
})
great to hear you found a good way!
Hey, i was using option.match like this today:
However, the problem is that the None function returns an
JSX.Element
, which forces the Some function to then also return aJSX.Element
, while i actually want it to return an arraySo what i propose is to change the types at https://github.com/emmanueltouzery/prelude-ts/blob/master/src/Option.ts#L594 to have two generics, one for each function accepted
I made a typescript playground of it: https://www.typescriptlang.org/play/index.html#src=type%20Option%3CT%3E%20%3D%20None%3CT%3E%20%7C%20Some%3CT%3E%3B%0D%0A%0D%0Aclass%20None%3CT%3E%20%7B%0D%0A%20%20%20%20constructor()%20%7B%20%7D%0D%0A%0D%0A%20%20%20%20match%3CU%2C%20V%3E(cases%3A%20%7BSome%3A%20(v%3A%20T)%3D%3E%20U%2C%20None%3A%20()%20%3D%3E%20V%7D)%3A%20U%20%7C%20V%20%7B%0D%0A%20%20%20%20%20%20%20%20return%20cases.None()%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Aclass%20Some%3CT%3E%20%7B%0D%0A%20%20%20%20constructor(private%20value%3A%20T)%20%7B%20%7D%0D%0A%0D%0A%20%20%20%20match%3CU%2C%20V%3E(cases%3A%20%7BSome%3A%20(v%3A%20T)%3D%3E%20U%2C%20None%3A%20()%20%3D%3E%20V%7D)%3A%20U%20%7C%20V%20%7B%0D%0A%20%20%20%20%20%20%20%20return%20cases.Some(this.value)%3B%0D%0A%20%20%20%20%7D%0D%0A%7D%0D%0A%0D%0Aconst%20foo%20%3D%20new%20Some(10).match(%7B%0D%0A%20%20%20%20Some%3A%20()%20%3D%3E%20123%2C%0D%0A%20%20%20%20None%3A%20()%20%3D%3E%20%22123%22%0D%0A%7D)%20%2F%2F%3D%3E%20string%20%7C%20number%0D%0A%0D%0Aconst%20bar%20%3D%20new%20Some(10).match(%7B%0D%0A%20%20%20%20Some%3A%20()%20%3D%3E%20123%2C%0D%0A%20%20%20%20None%3A%20()%20%3D%3E%20123%0D%0A%7D)%20%2F%2F%3D%3E%20number%0D%0A%0D%0A