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

depercate bs.send.pipe in the future #2625

Closed
bobzhang opened this issue Mar 15, 2018 · 13 comments
Closed

depercate bs.send.pipe in the future #2625

bobzhang opened this issue Mar 15, 2018 · 13 comments

Comments

@bobzhang
Copy link
Member

bobzhang commented Mar 15, 2018

The work around is changing code from

external ff : int -> int = "" [@@bs.send.pipe : t]

to

external ff : t -> int -> int = "" [@@bs.send]
let ff a b = ff b a 

With such change, it will not affect end users (slightly slower), eventually we want users just use bs.send with the new pipe syntax

external ff : t -> int -> int = "" [@@bs.send]
let () = a |. ff 2

The motivation is to reduce the complexity of FFI and maintainance overhead, but also encourage people to embrace t first convention

@chenglou
Copy link
Member

reduce the complexity of FFI

That'd be fantastic

@banacorn
Copy link

banacorn commented Jan 8, 2019

When should we use the OCaml |> and when should we use the pipe syntax |. (or ->) ?

For API providers: which one should I encourage the consumers to use?
For API consumers: which one should I use, when both are applicable?

@glennsl
Copy link
Contributor

glennsl commented Jan 8, 2019

I would recommend avoiding the fast pipe entirely, since it mostly just causes confusion. But especially if you're writing anything not exclusively targeting BuckleScript, since the fast pipe is a BuckleScript-exclusive weirdness.

Using the fast pipe prevents you from designing functions in a way that encourages function composition through partial application. For example, if you want to extract all the values from a list of options while providing a default value for those that are None, you can use List.map over Option.getWithDefault. With functions designed for the fast pipe, by putting the "target" argument first, you have to write:

/* Reason */
items |. List.map(item => item |. Option.getWithDefault("default"));
(* OCaml *)
items |. List.map (fun item -> item |. Option.getWithDefault "default");

But with functions designed for the normal pipe operator, by putting "t last", you can write:

/* Reason */
items |> List.map(Option.getWithDefault("default"));
(* OCaml *)
items |> List.map (Option.getWithDefault "default");

because Option.getWithDefault("default") is partially applied and will due to currying return a function that matches what List.map expects.

This example is pretty trivial, but the gains quickly add up. There are very good reasons why functional languages have mostly standardized on |>, and why even those that don't have a standardized pipe operator (like Haskell) still use "t last" and are curried in the first place.

@banacorn
Copy link

banacorn commented Jan 8, 2019

Thanks! Now |> makes good sense for me if I'm working with those standard libraries.

But what if I'm writing JS FFI bindings (hence exclusively BuckleScript), should I put the t in the front or as the last argument?

It seems like there are people advocating and converting their code with this "object-first convention" to make use of the BuckleScript fast pipe.

@glennsl
Copy link
Contributor

glennsl commented Jan 8, 2019

I would still recommend not using fast pipe. There is a performance benefit to using |. over |>, but there's no performance benefit over not using any pipe operator, when performance is actually needed. The problem with the fast pipe is that it forces a performance by default design that is very rarely beneficial, just for the sake of premature optimization.

Take it from Donald Knuth:

Programmers waste enormous amounts of time thinking about, or worrying about, the speed of noncritical parts of their programs, and these attempts at efficiency actually have a strong negative impact when debugging and maintenance are considered. We should forget about small efficiencies, say about 97% of the time: premature optimization is the root of all evil. Yet we should not pass up our opportunities in that critical 3%.

In web front-end programming especially, the time spent in code you write is usually dwarfed by the time spent updating the DOM.

@banacorn
Copy link

banacorn commented Jan 9, 2019

external ff : t -> int -> int = "" [@@bs.send]
let ff a b = ff b a 

I don't really mind the performance cost.
However, it's the need of the extra line that makes it painful, and isn't that what [@@bs.send.pipe : t] was meant for?
@bobzhang what should we do about this?

@bobzhang
Copy link
Member Author

bobzhang commented Jan 9, 2019

We plan to make the external FFI handling logic simplified in the core, but it is not done since it may break some existing code. bs.send.pipe is the single attribute which will tweak the type signature, so it also complicates a bit for the user.
There are lots of benefit using fast pipe, but people may disagree, so I would not trigger a flaming war here.

@glennsl
Copy link
Contributor

glennsl commented Jan 9, 2019

I would love to see those lots of benefits clearly explained sometime. The only rationale we've gotten so far, as far as I'm aware, is this comment and the thread that follows, which 1. doesn't really explain much and 2. ignores any criticism and concern raised.

You can't expect anyone to "agree" with you when you go about it like that, and I don't think it's going to help your cause to characterize those who therefore don't understand, or don't agree with whatever assumptions you're making, as potential flamers.

@bobzhang
Copy link
Member Author

bobzhang commented Jan 10, 2019

@glennsl I think we already talked about it offline for more than several hours.
Let me summarize it below (other people may not know details about it)

  • t last convention can not be formally defined in a curried language.
    people can not tell whether an arg is last or not by just looking at types.
    Take 'a -> 'b -> ('c -> 'd) for example, in t-last convention, it is hard to tell if 'c is t or 'b.
    In t-first convention, it is pretty clear 'a is t
  • t last convention is in conflict with bi-arguments, compare, startsWith
  • t last convention is in conflict with JS API convention, that's why I introduced bs.send.pipe which adds a lot of complexity compared with bs.send alone
  • |> is less efficient than |. in both compile time and runtime
  • |> introduced more type annotation than |. due to the ocaml type inference algorithm
  • |> introduced slower code compared with |. (esp true when we upgrade to OCaml 4.06) due to OCaml type information flow direction
  • The advantage that |> makes currying easier is arguable, it is not my own view: accidental currying is harmful
  • The argument that |> is widely used is arguable, janestreet adopted t first convention, even in OCaml stdlib, it is not consistent, take compare for example

I already spent lots of time explaining this issue, let's agree to disagree.

  • |> will always be preserved
  • bs.send.pipe will not be removed in the near to mid term

@glennsl
Copy link
Contributor

glennsl commented Jan 10, 2019

A summary of what? This is just a reiteration of your original points, ignoring any criticism and concern that has been raised. You are intentionally misrepresenting the issue.

Here's a summary of the other side:

  • Your first point doesn't seem to be an issue in practice. I've asked for examples to illustrate the problem, but you've refused to provide any.
  • Conflicting bi-arguments has nothing to do with t first or last. In both cases it can and should be solved using labeled arguments.
  • |> is less efficient than |.. This is the only argument that seems to hold up. However, it's not less efficient than not using any pipe operator at all when that extra bit of performance is actually needed. |> could also be made just as efficient as |. by treating it as a syntactic transform, at the small cost of changing order of evaluation, which is more of a theoretical than a practical problem.
  • Type inference is certainly different with t first, but not necessarily better. It introduces some other inference issues too.
  • I have no idea what "accidental currying" is, because you refuse to explain it, but I can't see how |. could prevent anything like that.
  • Jane Street adopted t first before there was a pipe operator, but also did so in a way that still supports partial application and the normal pipe operator. Because they use labeled arguments. Several people asked for that as a compromise, but you flatly rejected it for being "too verbose".
  • The OCaml stdlib is inconsistent. Period. It has nothing to do with t first or last.
  • Having both |> and |. causes a lot of confusion about which to use when, especially when you have to continuously switch between them. Having to remember what "kind" of function each and every function is in order to use the correct pipe operator on them is a heavy cognitive burden. This alone is a major issue that deserves proper justification, but you haven't even acknowledged that it is an issue yet.

What I'm asking for is a clear and thorough explanation that looks at all sides. Despite all the hours you say you have spent explaining this, you seem to be incapable of doing that. Why is that? Do you just not care?

@ozanmakes
Copy link

Mentioning Jane Street's practice of "t first" without mentioning they only use a single positional argument is indeed a bit misleading. They use the classic pipe operator everywhere.

I'd love to have a BeltLabels module, you can take a look at Containers for an example of a stdlib that automatically generates both conventions: c-cube/ocaml-containers#233

|> could also be made just as efficient as |. by treating it as a syntactic transform

See https://github.com/janestreet/ppx_pipebang

@chenglou
Copy link
Member

Enough is enough.

@rescript-lang rescript-lang locked as too heated and limited conversation to collaborators Jan 10, 2019
@cknitt
Copy link
Member

cknitt commented May 2, 2023

@bs.send.pipe was removed in ReScript 10.0.

@cknitt cknitt closed this as completed May 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants