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

Changing the function composition operator link #64

Closed
dead-claudia opened this issue Oct 10, 2017 · 40 comments
Closed

Changing the function composition operator link #64

dead-claudia opened this issue Oct 10, 2017 · 40 comments

Comments

@dead-claudia
Copy link
Contributor

dead-claudia commented Oct 10, 2017

Edit: Added links.
Edit 2: Here's a comment clarifying some of the high points of the below thread.

This thread pretty much ruined any and all hopes I had in that particular proposal, where the OP doesn't seem particularly receptive to the criticism he's receiving of his proposal (and I'm not quite convinced he has much command of the subject matter).

Would it be wise at this point to change the function composition operator link to point to another proposal? (Shameless plug: I have my own separate proposal, but mine isn't the only alternative - there's others that are effectively the same thing, including function-based and method-based variants.)

@TehShrike
Copy link
Collaborator

I think I agree on all counts. What are some of the other proposals (i.e. why didn't you throw in behind one of the others)? Maybe we could link to a couple.

@dead-claudia
Copy link
Contributor Author

dead-claudia commented Oct 10, 2017

@dead-claudia
Copy link
Contributor Author

(I updated the original issue accordingly.)

@dead-claudia
Copy link
Contributor Author

Although, it appears that one has dead links in it, and one of those linked there is the proposal in question I'm requesting to be de-linked.

@dead-claudia
Copy link
Contributor Author

dead-claudia commented Oct 10, 2017

I finally found a few decent links to add. (They're really scattered, and not very many of us actually put together a serious gist/repo.)

@TheNavigateur
Copy link
Contributor

As the author of the existing link, of course I disagree. Happy to defend the proposal in full. The crux of the discussion is whether the operator should allow direct async function composition from one or more other async functions. The proposal allows intuitive composition of any of the 4 available function types: functions, async functions, generator functions and async generator functions from other functions, async functions, generator and/or async generator functions. Join the discussion here: TheNavigateur/proposal-pipeline-operator-for-function-composition#3 (comment)

@dead-claudia
Copy link
Contributor Author

dead-claudia commented Oct 11, 2017

@TheNavigateur Find me on Gitter (you may need to sign up and search for @isiahmeadows, my username) or ping me via email (me@isiahmeadows.com), and I'll happily explain the issue further. I'd rather explain this one privately why I, and others I've talked to privately, have drawn issue with the way you handled/responded in that thread.

@littledan
Copy link
Member

There's a huge amount to read through here. Can anyone provide a concise summary?

@dead-claudia
Copy link
Contributor Author

@littledan There's a lot of noise in the thread, but here's the highlights:

  1. The repo's owner has not really addressed some of the questions at hand, such as this concern of mine regarding the syntax being a little too subtle, and the premise of the bug, which calls into question implicit promise resolution.
  2. He doesn't seem to understand promises very well, which makes it pretty hard to create a decent, workable, useful proposal.
  3. He seems to have believed that there's a meaningful callee distinction between async functions and promise-returning ones, when there really isn't. Furthermore, maintaining such a distinction has been explicitly pointed out to him as problematic without response.
  4. Most of the thread from here down has been focused on his own persistent confusion caused by 2 and 3, and his responses have almost continually been reiterating the same confusion, despite repeated attempts to clarify and explain what we were talking about.

@littledan
Copy link
Member

OK, let's leave personal issues aside. What should I understand about the issue, as a TC39 champion?

@dead-claudia
Copy link
Contributor Author

dead-claudia commented Oct 11, 2017

@littledan An alternative would be to include links to other similar proposals such as mine and the other I linked to.

Edit: If you'd prefer, I can close this and re-file a new bug with just that part.

@mAAdhaTTah
Copy link
Collaborator

If this is just a suggestion for the README, can you just PR the change and we can move on? The issues in that repo don't need to spill over here, and shouldn't have any impact on this proposal.

@TehShrike
Copy link
Collaborator

@littledan nothing related to this proposal - only the sidenote part of the readme. That sidenote section of the readme only exists to point people somewhere else so they don't introduce unnecessary repetitive red-herring issues here.

@TehShrike
Copy link
Collaborator

The obvious choices are to either say "if you want a function composition operator, that's something else, go find it" or "here's a list of function composition proposals, if that's what you want".

The latter may be slightly more effective at preventing people from opening more issues against this repo? Not sure. Given this issue, maybe not.

@TheNavigateur
Copy link
Contributor

TheNavigateur commented Oct 12, 2017

@littledan Should async functions introduced into the composition, such as async input=>output, pipe output to the next function and produce an async composed function, or should it pipe aPromise to get output the next function and still produce a synchronous composed function? Proposal says async and output. Issue says Promise to get output and synchronous function. This is the essence of the disagreement in the issue.

The proposal uses the same principle for generator functions and async generator functions (intuitive piping). 2 reasons: power and bug minimization in accomplishing such tasks in the problem space. Overloading obviously has precedence in + e.g. myNumber+'myString' evaluates to a string, and is also used very intuitively.

@littledan
Copy link
Member

I think async is potentially in scope; is there a particular concrete proposal I should review more in depth?

I agree with keeping function composition out of scope and linking to proposals for it in the readme. Is there a PR which does this?

@TheNavigateur
Copy link
Contributor

TheNavigateur commented Oct 12, 2017

@littledan Of course do review https://github.com/TheNavigateur/proposal-pipeline-operator-for-function-composition .

Yes of course function composition is out of scope for this pipeline operator proposal. They can peacefully coexist

@littledan
Copy link
Member

@TheNavigateur If you can make a PR against this repo to link to it from the explainer, I can merge that PR.

Is there a proposal I should look at for async integration with |> ?

@TheNavigateur
Copy link
Contributor

@littledan It's already linked to in the README of this proposal.

As for async with |>, no change is required:

syncFunction1 |> await asyncFunction1 |> syncFunction2

@littledan
Copy link
Member

OK, if it's already linked, what's the problem? I agree that we shouldn't put composition in this proposal. Are there any more that we should link as alternatives?

I'm not sure what you mean about async with |>; that code won't do what you're imagining, as I explained elsewhere--it will just await the function itself, not await calling the function on the argument. Should we change that?

@TheNavigateur
Copy link
Contributor

TheNavigateur commented Oct 13, 2017

@littledan Yeah, then I would change it so that await awaits calling the function on the argument before it. But that's just my opinion.

@kurtmilam
Copy link

@isiahmeadows proposed linking to a different composition operator proposal, or, as a fallback option, adding links to alternative proposals. He was floating the idea for comment before issuing a PR.

Two alternative proposals are:
https://github.com/isiahmeadows/function-composition-proposal
https://github.com/simonstaton/Function.prototype.compose-TC39-Proposal

Isiah's proposal seems solid to me. He started working on it more than a year ago and has been maintaining it actively in the interim.

The concern (shared by others) is that the link from the official TC39 pipeline operator proposal gives the appearance of TC39's imprimatur to the currently-linked (and potentially flawed w/r/t to handling Promises and async functions) composition operator proposal.

Pinging @simonstaton (the author of the second alternative proposal) for input.

@TheNavigateur
Copy link
Contributor

@TehShrike I would read it as follows: Pass x to the function then await for it to complete, then pass its declared return value to the next function:

x |> await f |> g

Were you thinking of doing it a different way?

@TheNavigateur
Copy link
Contributor

@kurtmilam Just to be clear, the composition proposal doesn't handle promises, only async functions (and generator functions, and async generator functions). It's purely function composition

@littledan
Copy link
Member

OK, if someone makes a PR which has a list of links to proposals, replacing a single link, I'd merge that.

@kurtmilam
Copy link

@littledan I'll give @simonstaton (alternative proposal author) time to respond.

I'd like to know whether he plans to maintain his proposal and would like to have it linked from this one before I issue a PR.

@TheNavigateur
Copy link
Contributor

TheNavigateur commented Oct 13, 2017

I'm not sure it matters whether he will maintain it. If it's a good idea it can be forked and maintained by anyone interested, as it is a public repo. I've created a pull request anyway, that can be pulled whenever deemed appropriate, either way

@ljharb
Copy link
Member

ljharb commented Oct 13, 2017

To clarify #64 (comment): async functions are promise-returning functions; so it either handles both or neither.

@kurtmilam
Copy link

async functions are promise-returning functions; so it either handles both or neither.

@ljharb several people tried patiently (and unsuccessfully) to explain that to @TheNavigateur . The unsuccessful outcome of that conversation is what spurred @isiahmeadows to open this issue.

@TheNavigateur
Copy link
Contributor

@ljharb Obviously async functions are promise-returning functions. But they are not themselves promises. That's all I'm saying. The proposal doesn't support Promise as an operand, but an AsyncFunction (as well as Function, GeneratorFunction and AsyncGeneratorFunction). This may be obvious anyway, but I'm just making it even more obvious by saying it

@ljharb
Copy link
Member

ljharb commented Oct 13, 2017

@TheNavigateur gotcha; of course you're correct, but it wasn't clear to me based on your earlier statement.

To restate: await applies to any function that returned a thenable, "async function" is utterly irrelevant from outside the function.

@TheNavigateur
Copy link
Contributor

@ljharb Just to correct you very slightly, await applies to a thenable itself, not to a function that returns one.

@TheNavigateur
Copy link
Contributor

@TehShrike That doesn't mean x |> await f would be inappropriate: it would be parsed in the case of the pipeline operator the same way as await f(x)

@TheNavigateur
Copy link
Contributor

@ljharb
Copy link
Member

ljharb commented Oct 14, 2017

@TheNavigateur in the context of this proposal and thread, it would theoretically apply to a function that returns a thenable.

@TheNavigateur
Copy link
Contributor

@ljharb When I wrote x |> await f, I wasn't really thinking about await f as a self-contained expression (which would be awaiting the function itself, which is invalid). Rather, I was thinking about it being parsed in the case of |> the same way as await f(x), which is awaiting the thenable returned by the function (which is valid). Yes, this would work perfectly for any function that returns a thenable.

@littledan
Copy link
Member

@TheNavigateur What you're saying is a possible alternative proposal, but that's not what the current spec text says.

@TheNavigateur
Copy link
Contributor

TheNavigateur commented Oct 14, 2017

@littledan You're absolutely right of course, and I realized that immediately upon your response to when I first wrote it. And of course yes, I would simply propose it as the way to do await with |>

@TheNavigateur
Copy link
Contributor

@littledan I've created a pull request for the await idea, for review

@dead-claudia
Copy link
Contributor Author

I'm closing this bug in favor of the more useful #67.

@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

7 participants