Various small fixes

[OliverJAsh]

Re. https://github.com/unsplash/unsplash-web/pull/3814#issuecomment-507207611

[OliverJAsh]

Something for the future: test the type output

[karol-majewski]

Might be something we could take a stab at as @WrocTypeScript/organisers for the purpose of doing WrocTypeScript/talks#19.

I'm sure many have the same problem.

[OliverJAsh]

Only used by tests, so safe to move here

[karol-majewski]

Precisely. Reference tweet for anyone interested:

https://twitter.com/bterlson/status/1100797304632557573

[OliverJAsh]

This is why I initially skipped adding an overload for "any number of functions", because it loses type safety. 🤔

[karol-majewski]

Which is the correct way to go. My concern was not about the upper limit (9+) but the lower one.

pipe and pipeWith should be allowed to accept parameters dynamically, and as such, they should work with an empty list of arguments or with a single function passed as a tuple.

The original motivating example:

declare const functions: [add1] | [add1, multiplyBy2] | [add1, multiplyBy2, toString];

pipeWith(
  0,
  ...functions, // Unexpected compile-time error
)

[karol-majewski]

We could also suggest adding overloads locally in the README if one really needs them to keep the core small and maintainable.

declare module 'pipe-ts' {
  export function pipeWith<A, B, C, D, E, F, G, H, I, J, K>( /* ... */);
}

[OliverJAsh]

My concern was not about the upper limit (9+) but the lower one.

Are you saying we don't need the 10+ overload? I think we do, to support the array use case?

[karol-majewski]

We could run a BigTSQuery to see how popular this pattern is (strongly typed tuples) or see how many overloads others libraries are offering and do the same.

[karol-majewski]

compose from Redux bails out after 4 functions:

https://github.com/reduxjs/redux/blob/d4075e111612a7bfb5a1fbeabc219ae81c4eb099/index.d.ts#L549-L578

Sanctuary can manage 5 functions:

https://github.com/karol-majewski/DefinitelyTyped/blob/5f59f3dc7a0965fb1767cf13c670c633bbbbf0a8/types/sanctuary/index.d.ts#L184-L189

Ramda does 10 functions:

https://github.com/karol-majewski/DefinitelyTyped/blob/5f59f3dc7a0965fb1767cf13c670c633bbbbf0a8/types/ramda/index.d.ts#L2093-L2103

I think what 10 should be enough + we need a bailout strategy just like Redux does with compose (although they make poor use of type parameters here).

export function compose<R>(
  f1: (b: any) => R,
  ...funcs: Function[]
): (...args: any[]) => R

[OliverJAsh]

My worry about having a "bailout" array overload is that type safety will silently disappear once you get past a certain threshold, e.g. I have a pipe with 9 functions, I add another one, and now suddenly I'm into any/unknown territory. I guess that's OK as long as we use unknown and not any (as I currently am in this PR)?

Or would it be better to do away with this bailout overload? If we did that, would the use cases you're concerned about (tuples up to 9 items in length) still work?

[karol-majewski]

I guess that's OK as long as we use unknown and not any (as I currently am in this PR)?

Exactly!

Or would it be better to do away with this bailout overload?

We need to draw the line somewhere.

• If we decide not to add the bailout one at all, it won't compile for people using more than 10. They will need to assert both their arguments (because there will be no matching overload) and the final return type.
• If we do add it, the consumers will need to assert the return type (because they will be given unknown).

This might look stupid, but if we are to allow the operator functions to be passed dynamically, then these two would be justified:

No functions

export declare function pipeWith<A>(a: A): A;

A lot of functions

export declare function pipeWith(a: any, ...functions: Function[]): unknown;

With these in place, it works like this:

const add = (x: number) =>
  (y: number) =>
    x + y;

const add5 = add(5);

const zero = [] as const;
const one = [add5] as const;
const five = [add5, add5, add5, add5, add5] as const;
const ten = [add5, add5, add5, add5, add5, add5, add5, add5, add5, add5] as const;
const twelve = [add5, add5, add5, add5, add5, add5, add5, add5, add5, add5, add5, add5] as const;

pipeWith(0, ...zero);   // $ExpectType 0
pipeWith(0, ...one);    // $ExpectType number
pipeWith(0, ...five);   // $ExpectType number
pipeWith(0, ...ten);    // $ExpectType unknown
pipeWith(0, ...twelve); // $ExpectType unknown

[OliverJAsh]

• If we decide not to add the bailout one at all, it won't compile for people using more than 10. They will need to assert both their arguments (because there will be no matching overload) and the final return type.

Or:

• Add an overload locally
• Send a PR to add an overload upstream
• Nest pipes! - pipe(...8Fns, pipe(...another9Fns)) 😆

Also worth mentioning that I've never had a tuple of functions longer than 9 before, and I've used pipe for a long time :-P

FWIW I had to remove the bailout/array overload due to a TS bug: #3. So we can continue this discussion but keep in mind that right now it's a non-option!

[karol-majewski]

Also worth mentioning that I've never had a tuple of functions longer than 9 before, and I've used pipe for a long time :-P

Exactly! I don't think it's much of an issue, and if it is, then the users will tell us about it.

[karol-majewski]

👍