effect icon indicating copy to clipboard operation
effect copied to clipboard

Suggestion: improve DX of type errors from inside `pipe` and `flow`

Open OliverJAsh opened this issue 1 year ago • 3 comments

What is the problem this feature would solve?

When writing code inside pipe and flow we frequently run into scenarios where a definition is missing e.g. because it hasn't been imported yet. This can result in very confusing error messages due to TypeScript inferring unknown for the type parameters.

Here's a small example comparing the behaviour with and without pipe:

// Deliberately not defined
// declare const add: (x: number) => (n: number) => number;

declare const identity: <T>(x: T) => T;

declare function pipe<A, B, C>(a: A, ab: (a: A) => B, bc: (b: B) => C): C

// ## Arrow function with block body

// ### without pipe

// ✅ Error is clear
// ✅ Just one error
(): number => identity(add(1)(1));

// ### with pipe

// ❌ Two errors instead of one
(): number => {
  return pipe(1, add(1), identity);
};

// ## Arrow function with expression body

// ❌ Two errors instead of one
// ❌ The most important error is masked ("Cannot find name 'add'") by another
// error ("Type 'unknown' is not assignable to type 'number'.")
(): number => pipe(1, add(1), identity);

This is especially problematic inside large pipelines. In my experience it's not uncommon for TypeScript to report multiple errors and highlight hundreds of lines of code just because of one missing import/definition.

What is the feature you are proposing to solve the problem?

This isn't a problem when we're not using pipe or flow:

// Deliberately not defined
// declare const add: (x: number) => (n: number) => number;

declare const identity: <T>(x: T) => T;

// ✅ Error is clear
// ✅ Just one error
(): number => identity(add(1)(1));

In this case, TypeScript infers the return type of add as any, so there are no further errors.

This is the DX we want for pipe and flow: there's just one error and it's clear what and where the error is.

How can we mimic this behaviour with pipe/flow? We can use default type parameters:

-declare function pipe<A, B, C>(a: A, ab: (a: A) => B, bc: (b: B) => C): C
+declare function pipe<A, B = never, C = never>(a: A, ab: (a: A) => B, bc: (b: B) => C): C

Complete example:

// Deliberately not defined
// declare const add: (x: number) => (n: number) => number;

declare const identity: <T>(x: T) => T;

declare function pipe<A, B = never, C = never>(a: A, ab: (a: A) => B, bc: (b: B) => C): C

// ## Arrow function with block body

// ### without pipe

// ✅ Error is clear
// ✅ Just one error
(): number => identity(add(1)(1));

// ### with pipe

// ✅ Error is clear
// ✅ Just one error
(): number => {
  return pipe(1, add(1), identity);
};

// ## Arrow function with expression body

// ✅ Error is clear
// ✅ Just one error
(): number => pipe(1, add(1), identity);

I picked never instead of any because it felt safer. 🤷‍♂️

What alternatives have you considered?

For arrow functions with an expression body, TypeScript could adjust the position of the return type error so it doesn't cover the entire expression: https://github.com/microsoft/TypeScript/issues/57866.

However, this wouldn't completely fix the problem. pipe and flow would still infer their type parameters as unknown, resulting in extraneous errors.

OliverJAsh avatar Jul 15 '24 19:07 OliverJAsh

@mikearnaldi I'm curious if you or the team have any thoughts on this? I would be happy to send a PR.

OliverJAsh avatar Aug 06 '24 14:08 OliverJAsh

@OliverJAsh I think that a PR (with all the type tests passing) would be able to push this over the finish line smoothly. Are you still up to champion this?

datner avatar Sep 05 '24 00:09 datner

@datner https://github.com/Effect-TS/effect/pull/3549

OliverJAsh avatar Sep 05 '24 08:09 OliverJAsh

Fixed by https://github.com/Effect-TS/effect/pull/3731.

OliverJAsh avatar Nov 05 '24 14:11 OliverJAsh