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.

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

Prefer flow #97

Open
thewilkybarkid opened this issue Mar 24, 2021 · 5 comments
Open

Prefer flow #97

thewilkybarkid opened this issue Mar 24, 2021 · 5 comments

Comments

@thewilkybarkid
Copy link

thewilkybarkid commented Mar 24, 2021

If the argument for a pipe is only referenced in the first step, it can be replaced with a flow:

(foo) => pipe(
  foo,
  ...
)

and

(foo: FooType) => pipe(
  foo.bar,
  ...
)

can be:

flow(
  ...
)

and

flow(
  (foo: FooType): foo.bar,
  ...
)

Can be easier to read, and keeps it out of scope for the rest of the pipeline.

Real examples

@thewilkybarkid
Copy link
Author

Aware I've opened quite a few suggestions, hoping to put some time aside to start implementing soon, assuming the approach in #64 is ok.

@gabro
Copy link
Member

gabro commented Mar 24, 2021

@thewilkybarkid thank you so much, I'm sorry about leaving you hanging on #64, I haven't had a minute to review it, since schools are closed in Italy and this complicated my personal situation a bit 😩

I hope I'll find a minute soon! Thanks for the patience

@gabro
Copy link
Member

gabro commented Mar 24, 2021

by the way, this specific suggestion may not be a safe refactor in 100% cases, due to type inference.

I don't have a specific example to bring, but it happens in many cases that refactoring from pipe to flow causes the inferred type of the expression to change.

The transform from a => foo(a) to foo is technically an eta-reduction, which is safe in theory, but not in TypeScript.

For this reason, I'm not fully convinced about implementing this as a rule/fix, since it may lead to unwanted results.

@thewilkybarkid
Copy link
Author

@thewilkybarkid thank you so much, I'm sorry about leaving you hanging on #64, I haven't had a minute to review it, since schools are closed in Italy and this complicated my personal situation a bit 😩

I hope I'll find a minute soon! Thanks for the patience

Absolutely no problem. Can commit some work time to have a crack at some of these, but don't just want to trigger the usual open-source problem of being demanding. 😅

by the way, this specific suggestion may not be a safe refactor in 100% cases, due to type inference.

I don't have a specific example to bring, but it happens in many cases that refactoring from pipe to flow causes the inferred type of the expression to change.

The transform from a => foo(a) to foo is technically an eta-reduction, which is safe in theory, but not in TypeScript.

For this reason, I'm not fully convinced about implementing this as a rule/fix, since it may lead to unwanted results.

It'd be interesting to find real cases where it is unsafe, and see if it's possible to exclude them...

@gabro
Copy link
Member

gabro commented Mar 24, 2021

one case I remember (but I don't have time to reproduce fully right now) is when working with Do/ bind in fp-ts: using point-free style (i.e. flow-like calls) there for the bind messes up the inference of the "scope" that's being carried on.

I've discussed this some time ago with @gcanti and I remember he being skeptical about this as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants