seasonedcc / composable-functions

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

🐛 Fix type bug with trace function #59

Closed gustavoguichard closed 1 year ago

gustavoguichard commented 1 year ago

Purpose

When doing type-level testing (#58) I realized the newly introduced trace function kills the type inference of the composition applied to it. Any DF given to trace will be inferred as DomainFunction.

That was introduced in favor of typing the result of the trace callback which kind of defeats the purpose bc neither are being properly inferred unless you give trace a manual type argument.

We might be able to keep both inferences with some TS contortionism which I could not take off the 🎩.

The problem actually seems impossible on the first look, it lies on the API decision:

const df = makeDomainFunction()(async () => 1)
//    ^? DomainFunction<Number>

const logErrors = trace(({ input, result, output }) => console.log({ input, result, output }))
//                                 ?^ unknown
// the type is unknown and how am I supposed to know what is going to be applied later?

const dfWithLog = logErrors(df)
// Too late to infer the typeof df

It seems to be an underlying issue with the chosen order of parameters with the curried API.

Possible solutions for the current API

What could be done without breaking the API is to manually set the type param:

type TraceCallback<T> = ({ input: unknown, output: unknown, result: T }) => void

type Trace = <U = unknown>(
  cb: TraceCallback<U>
) => <T>(df: DomainFunction<T>) => DomainFunction<T>

const df = makeDomainFunction()(async () => 1)
//    ^? DomainFunction<Number>

const logErrors = trace<UnpackResult<typeof df>>(
  ({ input, result, output }) => console.log({ input, result, output })
//          ?^ Result<number>
)

Or even

type Trace = <U extends DomainFunction = DomainFunction<unknown>>(
  cb: TraceCallback<UnpackResult<U>>
) => <T>(df: DomainFunction<T>) => DomainFunction<T>

const logErrors = trace<typeof df>(
  ({ input, result, output }) => console.log({ input, result, output })
//          ?^ Result<number>
)

New API proposals

IMO I'd go this route.

We could have both inferences if we changed the trace API to something like:

type Trace = <T>(df: DomainFunction<T>) => (cb: TraceCallback<Result<T>>) => DomainFunction<T>
// or even this to keep same parameter order
type Trace = <T>(cb: TraceCallback<Result<T>>, df: DomainFunction<T>) => DomainFunction<T>

What do you think? We could bump the minor to 1.6 and deprecate the 1.5 package maybe.

diogob commented 1 year ago

We decided to keep the current implementation and add an optional generic to allow the user to overwrite the Result<unknown>.

In case you want to do that just pass the generic to trace as:

const logErrors = trace<DomainFunction<number>>(
  ({ input, result, output }) => console.log({ input, result, output })
//          ?^ Result<number>
)