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

Pipeline with spread operator? #61

Closed
hakatashi opened this issue Oct 5, 2017 · 3 comments
Closed

Pipeline with spread operator? #61

hakatashi opened this issue Oct 5, 2017 · 3 comments

Comments

@hakatashi
Copy link

hakatashi commented Oct 5, 2017

As mentioned in #23 (comment), this proposal will be more powerful when it supports spread operator .... In short, make something like the following

...[100, 200] |> Math.max

equivalent to the following.

Math.max(...[100, 200]) //=> 200

This change will fantastically make the pipeline able to pass more than one argument to the callee.

I suspect whether this would be legal when it chained more than one times. Obviously the following is valid,

...(...[10, 20] |> ((a, b) => [a + 5, b + 5])) |> console.log

but it would be nice to have something like the following

...[10, 20] |> ...((a, b) => [a + 5, b + 5]) |> console.log

and it's equivalent to the following.

console.log(...(((a, b) => [a + 5, b + 5])(...[10, 20]))) // prints '15 25'

Note that, since the spread operator is not a general unary operator and something like foo = ...bar; is not permitted syntactically, I think this kind of extension will result in a syntax extension of spread operator itself.

@hakatashi hakatashi changed the title How the pipeline interacts with spread operator? Pipeline with spread operator? Oct 5, 2017
@nmn
Copy link

nmn commented Oct 10, 2017

I don't see a big reason for this as arrow function pretty much solve the issue already:

// ...[100, 200] |> Math.max
[100, 200]
  |> _ => Math.max(..._)

// ...(...[10, 20] |> ((a, b) => [a + 5, b + 5])) |> console.log
[10, 20]
  |> ([a, b]) => [a + 5, b + 5]
  |> _ => console.log(..._)
// console.log will also work correctly now. (It's not bound and would be missing a this in your example)

Adding the extra spread syntax is confusing to read, and the alternative is easier to read.
Using arrow functions also doesn't have a downside as VMs can optimize them as much as the spreads. In fact the newly added Babel plugin already optimizes simple arrow functions to remove the function call altogether.

ALSO:
If using an arrow function with a spread is too much for you, you could also easily make a helper higher order function.

const apply = (fn, ctx = null) => args => fn.apply(ctx, args);

// ...[100, 200] |> Math.max
[100, 200]
  |> apply(Math.max)

// ...(...[10, 20] |> ((a, b) => [a + 5, b + 5])) |> console.log
[10, 20]
  |> apply((a, b) => [a + 5, b + 5])
  |> apply(console.log, console)

Here the apply function will also help with context. I'm all for adding functions like this to JS eventually, but I don't think we should add syntax unless it actually helps.

@mAAdhaTTah
Copy link
Collaborator

I'm 👎 on this. This looks like too much magic and confusion for what can already be done with arrow functions inline, or using a helper function for.

@littledan
Copy link
Member

OK, seems like a lot of opposition to this proposal, so closing this issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2021
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

4 participants