Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Open
OliverJAsh opened this issue Jul 15, 2024 · 3 comments
Open

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

OliverJAsh opened this issue Jul 15, 2024 · 3 comments
Labels
enhancement New feature or request

Comments

@OliverJAsh
Copy link
Contributor

OliverJAsh commented Jul 15, 2024 •
edited
Loading

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: microsoft/TypeScript#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 OliverJAsh added the enhancement New feature or request label Jul 15, 2024
@OliverJAsh
Copy link
Contributor Author

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

@datner
Copy link
Member

datner commented Sep 5, 2024

@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?

OliverJAsh added a commit to OliverJAsh/effect that referenced this issue Sep 5, 2024
@OliverJAsh
Add failing tests
be16d3b 
For Effect-TS#3257.
OliverJAsh added a commit to OliverJAsh/effect that referenced this issue Sep 5, 2024
@OliverJAsh
Add default type arguments to pipe and flow
87192cb 
Fixes Effect-TS#3257.
@OliverJAsh OliverJAsh mentioned this issue Sep 5, 2024
Closed
5 tasks
@OliverJAsh
Copy link
Contributor Author

@datner #3549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve DX of type errors from inside pipe and flow OliverJAsh/effect
2 participants
@OliverJAsh @datner