seasonedcc / composable-functions

Types and functions to make composition easy and safe
MIT License
666 stars 14 forks source link

Creates a 'sequence' function which is like a pipe function but savin… #33

Closed gustavoguichard closed 2 years ago

gustavoguichard commented 2 years ago

…g the results along the way

We may need a better name for the function.

The proposal here is to have the ability to pipe domain functions but keeping the output of every step along the way. The Result is going to be a tuple, similar to the output of all.

Result<[
  UnpackData<typeof firstDomainFunction>,
  UnpackData<typeof secondDomainFunction>,
  UnpackData<typeof thirdDomainFunction>
]>
gustavoguichard commented 2 years ago

Sure, no problem, I think they'll emerge anyway 😉

gustavoguichard commented 2 years ago

I think this is what @matiasleidemer was asking for on the last remix-brasil meeting ;D

matiasleidemer commented 2 years ago

Either I'm missing something, or I can't recall what I said at the meeting 😅

I remember asking for a way to type-check (not in runtime) the output of a function as the input of the next in the pipeline. Something along the lines of:

const a = makeDomainFunction(z.number())(async (id) => id + 1)
const b = makeDomainFunction(z.string())(async (id) => id + '1')

const c = sequence(a, b)
// ---^
// Argument of type 'number' is not assignable to parameter of type 'string'.

About what you implemented here, it looks really cool. But I'm unsure about its usability, for instance: removing a function from a sequence would break implementations that access the data by the function's index.

const a = makeDomainFunction(z.number())(async (id) => id + 1)
const b = makeDomainFunction(z.number())(async (id) => id + 2)
const c = makeDomainFunction(z.number())(async (id) => id + 3)
const d = makeDomainFunction(z.number())(async (id) => id + 4)

const flow = sequence(a, b, c, d)
const result = await flow(0)

// (...)

console.log(result.data[2]) // -> 6
// if we remove a, b or c from the sequence, the line above will return
// something else, which can lead to unexpected behavior

Please take my opinion with a grain of salt, I haven't used remix-domains in real-life applications, so maybe my concern doesn't apply.

gustavoguichard commented 2 years ago

@matiasleidemer , got it! My bad. About your concern with sequence (only a draft, yet to be better named):

This already happens to pipe, if you remove one step of the "flow" it gives different results and might break the flow at all.

The difference between the proposed sequence method and the pipe one is that the former saves the output of every step of the flow while pipe only returns the latest step's output.

declare function pipe(...df: DomainFunction[]): Result<UnpackData<typeof lastDomainFunction>>
declare function sequence(...df: DomainFunction[]): Result<[
  UnpackData<typeof firstDF>,
  UnpackData<typeof secondDF>,
  UnpackData<typeof thirdDF>
]>

Yesterday we had an use case where we wanted to pipe the userId from the session through another function userOffer but we still wanted the userId for something else... pseudo code:

const result = await pipe(getUser, getUserOffer)(null, environment)
// ------^
// getUserOffer needs the output of getUser but we want to have access to both [user, offer] here
gustavoguichard commented 2 years ago

@diogob , consider it again, please.

We just hit another wall with composition of domain functions and I think sequence is the way out.

For context, consider we have 3 domain functions (a, b, and c) whereas the output of a is required as input of b and c:

const d = pipe(a, merge(b, c))
// { resultB, resultC }

The problem is that I also need the output of a in my view and don't want to rerun the function cause it is not idempotent. I tried this approach:

const identity = makeDomainFunction(z.object({}).passthrough())(async (data) => data)
const d = pipe(a, merge(b, c, identity))
// { resultB, resultC, resultA }

But resultA will lose type inference.

If I had the sequence method I could do it like so:

const d = sequence(a, merge(b, c))
// [{ resultA }, { resultB, resultC }]

And could go even further with the help of map and mergeObjects:

const d = map(sequence(a, merge(b, c)), mergeObjects)
// { resultA, resultB, resultC }

Any thoughts?

danielweinmann commented 2 years ago

+1 for this :)

I've had more use cases where I need all the results (sequence) than where I need only the last result (pipe) so far. I ended up moving away from DF composition in those use cases because using pipe wasn't an option.

diogob commented 2 years ago

@gustavoguichard @danielweinmann I have considered the identity solution and also did some sketches of my own, but there are some problems with the identity approach that point me towards the current proposed solution.

The main issue with identity is that we are not building an identity function in the DomainFunction, but rather an indentity of the Result set. And for the typing to work the Result would have to carry information about the DF input, so we can say "the type of the resulting DF is whatever it gets in its input".

Since that is a lot of hacking in our current types it seems reasonable to merge this PR as is so we add to our interfaces and later, if opportunity arises we just change its implementation to use more fundamental operations.

diogob commented 2 years ago

BTW, I'm not sure if you guys chose the name because of the Haskell function but it seems to fit the type and the semantics.