-
Notifications
You must be signed in to change notification settings - Fork 463
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
Call For API Review #2463
Comments
Thanks a lot for this @bobzhang it looks really great! My only concern is something I brought up yesterday on Discord: the usage of The
The main benefits from what I understand are:
I certainly see the benefits, but I wonder if they outweigh the costs in developer experience. I am very interested to know your opinion, as well as others in the community. Thanks again for your amazing work!! |
Side comment regarding I do agree though with all the issues listed above by @jchavarri |
@rafayepes Oh definitely, sorry I wasn't clear. I'm referring exclusively to functions that are not built to interop in any way with external JS code. |
Looks great Bob. Seems you're using some phantom type parameters in some types for safety? These will need to be documented carefully. Maybe a general overview of the technique to get everyone on the same page. |
Whooo! This is so important to help its adoption in my opinion. :-) I'll try to look it over when I get time. :-) |
per discussion we will provide both forEach : ('a -> uint) -> 'a t -> unit
forEachU (or forEach' or forEach_) : ('a -> unit [@bs]) -> 'a t -> unit @yawaramin it will be documented |
Any chance we can get modules with labelled functions? I use Happy to contribute if it would help ;) |
instead of using |
|
|
This looks awesome 😃 Will the reflection api ( |
A few more notes:
Range.list 0 5 = [0; 1; 2; 3; 4]
Range.array 0 5 = [|0; 1; 2; 3; 4|]
Range.list ~step:2 0 5 = [0; 2; 3]
Range.list ~inclusive:true 0 5 = [0; 1; 2; 3; 4; 5]
let even n = n mod 2 = 0
List.groupBy even (Range.list 0 5) = [
true, [0; 2; 4];
false, [1; 3]
]
|
@yawaramin there is I updated the docs page, a new round of review is appreciated.
|
I second @TheSpyder’s wish for labeled parameters. I much prefer functions with labels. For example, compare the following two signatures:
Labels are also recommended by the OCaml manual, whose suggestions I really like in this regard: https://caml.inria.fr/pub/docs/manual-ocaml/lablexamples.html#sec45 |
Typo: |
Lots of cool stuff in this library! Iterators/enumarations would be nice. For example: https://ocaml-batteries-team.github.io/batteries-included/hdoc2/BatEnum.html I’d also want streams, but I don’t know how they would fit into a world with iterators, Node.js, etc. |
Node has streams! |
Indeed, and the Web API has iterators (not very nice ones though): https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Iteration_protocols#The_iterator_protocol |
@jaredly I’d like streams + combinators or maybe just iterators + a stream that can optionally return an iterator. |
@yawaramin True. Maybe wrapping them would work. What I miss much in JS are combinators for iterators (zip, fold, etc.). |
updated the new feedback, will address it later
|
@bobzhang OK. What’s the reason for inverting the order of parameter compared to OCaml’s stdlib? There,
I’d stick with the name |
Having |
Agreed, bucklescript may compile to javascript, but if we wanted javascript's limitations then we would use javascript, not OCaml, and |
@rauschma re: At least, that is the idiomatic meaning. I haven't tried these functions, so I can't guarantee that these obey that behaviour, but I believe they do--Bob can confirm. Note that the existing OCaml stdlib functions are all still available since |
+1 on that, I think the main "target" in a function that would be used for piping should be the last argument in the function. For example, something that recently came up on discord a couple of times:
Having a thread-first piping operator wouldn't fix this issue really, it still needs to be the last argument--because having it as the last argument also means you can make use of currying, and it also means that you can compose functions together without having to flip arguments around |
I made an issue reasonml/reason#1452 explaining that why t comes first is better, note I am not alone, all janestreet libraries follow the same rules. |
To be fair, the Jane Street libraries use the ' I do want to remind everyone though that this change will not affect anyone outside of the BS/Reason standard library. The existing OCaml libraries are still available and are more featureful in newer OCaml version 4.06 which is also on the roadmap for BuckleScript. No matter what design the BS/RE library uses, it will still be one option among several (albeit the primary option for many). |
It is true that you can still use the existing stdlib. The new one is designed with performance in mind, I made an option for people who want to deploy efficient and smaller production code, but you don't have to (curry is essentially anti-perf) |
That decision was made five years before the pipe operator was even added to the standard library, which makes the argument more of an appeal to tradition than an appeal to authority (which isn't a very good argument either). I also don't get the performance argument. Not just because I don't know a single BuckleScript user (except its author, of course) who cares that much about performance, but you can still avoid partial application when needed with t last. |
Hi, I hear your opinions, note t comes first does not mean we can not use pipe syntax (take GoLang, Elixir for example, they did a pretty good job). Here are several reasons why t comes first, ordered by importance:
I would like to hear more about objective argument. Also I think pipe syntax has nothing to do with the order of t, that's why I made a proposal on Reason side, I would encourage you to chime in to comment on that issue reasonml/reason#1452 P.S. I used the janestreet codebase as an example not saying it is authority, but saying I am not just trying to be unique, I concur with their wisdom in this particular convention |
@bobzhang All of these arguments are valid. My complaint is that putting the positional parameter first doesn’t work well together with optional parameters. This is an interaction in
Can that be fixed? |
This can be fixed at the Reason side, if we do the [1, 2, 3] |> List.map(?, foo) |> List.filter(?, bar) It desugars to non-piped nested function syntax calls (the output doesn't change; |> is already specially compiled away in ocaml for perf). And since it's at the syntax level, no typing involved here and it's the semantic you think it is. This wouldn't even be a breaking change; if we detect no |
@rauschma in general, you don't use optional argument as last parameter, people used to add a unit argument
Note this works worse with pipe let f ?(x=1) z ?(y=2) =
x + y + z
(* let v = f 3 *)
let v = 3 |> f |
@bobzhang With pipe (and traditional OCaml style), you’d put all labeled parameters first, followed by all positional parameters. I’m unhappy that you then need an additional unit parameter even though you already have a positional parameter:
That is, rules for optional parameters become more complicated than they are now. |
Only optional parameter |
But all labeled (apart from primary parameter) seems common. You’d evaluate partially more often than expected:
You’d also need to add a unit parameter. In other words: as soon as you have a single optional parameter, you need a positional parameter at the end. |
@bobzhang Thanks for making some concrete arguments. I could still use some clarification though:
I don't understand what this means. The last argument isn't always the last argument? Are you referring to the last argument being polymorphic for example? Could you give an example?
Having multiple unlabelled arguments of the same type seems like bad style to me, and isn't a matter of
Imperative is important. That's why I use OCaml rather than Elm or perhaps PureScript. But I don't see why these styles should be consistent. On the contrary, I want them to be inconsistent because they're different paradigms, and I'd like that to be really obvious. We might be talking about different kinds of inconsistencies though. Giving examples would help.
Noted. There might be minor issues in edge cases I've never encountered in practice. There are also edge cases with
None of which have a pipe operator, and all of which (probably) uses other mechanisms for function chaining, such as having objects with methods that mutate internal object state. It is possible to add a syntactic pipe operator to support piping with
What's "accidental" currying? Can you give an example? |
@glennsl I think @bobzhang is referring to having the last argument be optional, making it ambiguous which argument should be "the last one", see his example. Also the rest of the issue explains the potential type errors that we can avoid. Totally guessing here. The "accidental currying" is, I think, that when you do @rauschma I think what you're complaining about is about how optionals + autocurrying work together inside ocaml. This isn't something Reason can fix today unfortunately. But I think @bobzhang's proposal makes things less ambiguous, and with his syntax proposal here we'll get back the nice benefits of the pipe operator :) |
I don't like But again, this is where having labels lets you choose which order you want to use. I even switch to I'm going to throw in a new requirement - I'd like the stdlib to be as easy to extend as the ocaml stdlib. Regardless of whether we get labels or not, I'm likely to want a few custom things in all of my projects (e.g. infix monadic operators) so I expect to wrap this in a library that re-exports most of it with additions. That's also where I can add labels if they aren't in the default package. This might seem like an obvious request, but the last work along these lines (bs-containers) wasn't extensible due to the use of |
I still don't see how that applies. That's a problem with
Already noted, but really doesn't seem very significant. I've never had an issue with it, and if I do, it seems easy enough to fix. Inconvenient, yes, but a splinter in your toe is usually not worth chopping off your leg for :P. It's still very much a valid argument that should be considered though.
The proposal you link just below fixes all that by making the pipe operator a pure syntactic transform. "Accidental currying" could also refer to accidental partial application, which can occur with or without the pipe operator. The
If what's implemented from that proposal is semantically (significantly) different from the standard library pipe operator, it also comes with some not so nice costs. More complexity, more concepts to learn, more cognitive load, familiar features behaving unexpectedly etc. The only proposal that seems like a clear win is to make the pipe operator, as it works today, a pure syntactic transform. And even that's arguable. |
We'll make the pipe operator great enough. Give us a few more days. Stay tuned! |
Can @bobzhang or someone respond to @glennsl 's comments? |
@Risto-Stevcev I think I already made it clear, the different views are mostly philosophic. For example, in a curried language, you can not tell which argument is the last one by reading its types. I am not against sugars, but I don't want to pay for superficial abstractions, hence, I made a proposal to reason syntax to make both happy. I also think it is a good chance for reason syntax to fix some semantics of original ocaml design, introducing a syntax for pipe which could be better optimized, better error message, etc. Thanks all for comments, let's ship a beta release and collect more feedback in practice |
Not sure where feedback is being collected in practice, but here's an anecdote from some code I was working on today: Belt.Option.(
Js.Dict.get(bundle##inputData, "property_status_list")
|> flatMap(_, Js.Json.decodeArray)
|> map(_, Js.Array.map(Js.Json.decodeString))
|> flatMap(_, Util.OptArray.sequence)
|> map(_, Array.to_list)
|> map(_, String.concat(","))
|> map(_, v => "/property-timelines?statuses" ++ v)
); I would consider this to not only be philosophic, but to be decidedly inconvenient in practice. As someone new to the ecosystem, it's also hard to remember that things that come from the OCaml stdlib (e.g. I'm also unconvinced by the claim that lambdas look better in the final position. If they get too long, you can always pull them out into a Anyway, that's just my two cents. |
Some of that is because Belt isn't finished; I assume it would eventually have it's own form of But in general, I agree. The arguments for |
@mlms13 are you aware of |
It is not very polished yet, but we won't add any new features basically
The preview is here: https://bucklescript.github.io/bucklescript-tmp/api/Bs.html
TODO:
Full coverage:
Leave hash based collection for later
Next:
Why:
t
comes first )Native story:
========= some suggestions
union src src1ofs src1len src2 src2ofs src2len dst dstofs cmp
...==================================================================
==> see below, we can give List.concatMany : 'a list array -> 'a list
==> array is a more fundamental building block, also consistent with bs.splice
zipBy
which seems betterI feel mapWithKey looks better, map is mapping value, here we supply one more argument
will do
In almost all other languages (except JS), unordered means unsorted..
========================================
The text was updated successfully, but these errors were encountered: