-
Notifications
You must be signed in to change notification settings - Fork 107
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
Optional chaining support for the pipeline operator? #159
Comments
I think that this should be a separate proposal, with a different token (e.g. |
I'm not tied to the syntax - I was just using it as an example. Admittedly, it's a bit ugly.
It seems orthogonal in the way private fields are orthogonal to public fields, so I'm not convinced. |
Given that |
@ljharb That would execute |
@mAAdhaTTah then |
Feasibly that works with F# as well, just needs to be wrapped in an arrow function. Regardless though, I'm also inclined to think this should be a separate, follow-on proposal rather than overloading the initial operator design. |
This would really bloat the proposal and it can be done in a separate proposal after this one has already been promoted to stage3. Does seem useful though. |
Happy to support anyone who wants to make a new proposal for this. Typical real-world usage example: Example usage: function loadFromCrossSessionStorage(): SavedData {
return (window.localStorage.getItem(DATA_KEY) ?> JSON.parse) ?? DEFAULT_DATA;
} Compare with required code: function loadFromCrossSessionStorage(): SavedData {
const savedData = window.localStorage.getItem(DATA_KEY);
if (savedData != null) {
return JSON.parse(savedData);
}
return DEFAULT_DATA;
} Compare with @ljharb's suggestion: function loadFromCrossSessionStorage(): SavedData {
return (window.localStorage.getItem(DATA_KEY) |> # != null ? JSON.parse(#) : #)
?? DEFAULT_DATA;
} |
Just my personal opinion, but I think this usage is getting just a bit too tricky. I worry that if we start adding many cute combinations like this, it will get hard to read JavaScript code. |
@littledan While I agree in general and I plan specifically not to suggest any further, I feel this one, optional pipeline chaining, to be generally and broadly useful enough to merit a single exception. This is about as far as I'd be okay with, and I agree anything more is just superfluous and unnecessary. For comparison, here's it compared to a hypothetical function loadFromCrossSessionStorage() {
return (window.localStorage.getItem(DATA_KEY) ?> JSON.parse) ?? DEFAULT_DATA;
}
function loadFromCrossSessionStorage() {
return window.localStorage.getItem(DATA_KEY)?.readJSON() ?? DEFAULT_DATA;
} Visually, it doesn't look that different, and semantically, the way a lot of people tend to use pipeline operators, it fits right into their mental model of them being "method-like". Separately, for language precedent:
So in summary, that simple operation is itself already pretty clumsy in languages that lack equivalent functionality, and it does in fact get harder to read IMHO the more often it appears. Personally, I find myself abstracting over this lack of functionality in JS a lot, and in a few extreme cases wrote a dedicated helper just for it. |
@isiahmeadows This is really a helpful analysis! Personally I don't like With the examples of other languages, I am increasingly convinced that we may need a separate Extension methods proposal to solve this problem. |
@isiahmeadows Thank you for the link. Actually now there are 5 different strategies for
There are already many discussion between last two options (F# vs Smart style), so no need to repeat them here. What I what to say is, many use cases of first option can never be meet with option 4 and 5. Especially option 5 can cover all use cases of 2,3,4 but not 1. I also think the main motivation of Extension methods is not method/function chaining, So I believe we can have both Extension methods and Pipeline op. Return to this issue itself (optional chaining), it's strange to have special pipeline op for it, but I feel it's acceptable to have |
@hax I agree that ( |
@littledan I concur w/ @isiahmeadows this is definitely an exception & would be immensely useful for those who are most likely intend to write in this style, functional programmers or reactive functional programmers. It would save a tremendous amount of time having to normalize and wrap over methods like |
I find using pipeline, I end up with tons of nullish checks. Lots of times, I really just want an Option, which just isn't that easy to work with in JS. Instead, we have null, and I think we should leverage that. To that end, I think if we build a proposal out of this, the operator should be |
It seems like support for optional chaining pipelining can be sort of cleanly separated from the rest of the pipeline proposal, and added as a follow-on. Is this accurate, or are there additional cross-cutting concerns? |
It can be a follow-on - it already depends on the pipeline operator making it. |
Has anyone taken to writing a proposal for this add-on? I'm doing game dev and this would greatly clean up my code. I could write collision detection as such:
Without this addition, I'd have to unpackage the how: And without the pipeline proposal, I'm actually using map, then filter, then foreach to build up what a potential collision is, filtering it if it didn't collide, then calling onCollision if it did. It reads nicely, but is less effecient. |
This might be going too far, but what about another extension using the array syntax:
which would be equivilent to:
Note: this is because pipeline works by chaining and if storeCollision doesn't return |
How about: const ifNotNull = f => a => a && f(a)
collidesHow(entityA, entityB) |> ifNotNull(onCollision) Not sure what it does to performance but since Also if you use |
Point-free is more ideal; or is that a suggestion for today prior to a
functioning shorthand like `?>` that was suggested?
On Sun, Jul 12, 2020 at 12:39 AM Johan ***@***.***> wrote:
How about:
const ifNotNull = f => a => a && f(a)
collidesHow(entityA, entityB) |> ifNotNull(onCollision)
Not sure what it does to performance but since ifNotNull(onCollision) has
to be resolved only once, and not every iteration, I think it couldn't hurt
too much...
Also if you use ifNotNull a lot, you could come up with a shorter name so
it doesn't add that many extra characters.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#159 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAJKUOFWJNO26D46OBDTRTTR3FSB3ANCNFSM4JN57JOQ>
.
--
Kevin Lozandier
lozandier@gmail.com <lozandier@gmail.com>
|
@Jopie64 let newScore = person
|> getScoreOrNull
?> double
|> (_ => add(7, _))
|> (_ => boundScore(0, 100, _));
// would become
let newScore = person
|> getScoreOrNull
|> ifNotNull(double)
|> ifNotNull(_ => add(7, _))
|> ifNotNull(_ => boundScore(0, 100, _));
// or nested pipelines
let newScore = person
|> getScoreOrNull
|> ifNotNull(_ => _
|> double
|> (_ => add(7, _))
|> (_ => boundScore(0, 100, _))
); |
True, I didn't think of short-circuiting... |
@noppa My proposed semantics would align with the optional chaining operator. let newScore = person
|> getScoreOrNull
?> double
|> (_ => add(7, _))
|> (_ => boundScore(0, 100, _))
// Would become
let newScore = person
|> getScoreOrNull
|> ifNotNull(_ => _
|> double
|> (_ => add(7, _))
|> (_ => boundScore(0, 100, _))
)
// Or as a statement
let score = getScoreOrNull(person)
let newScore = score != null ? boundScore(0, 100, add(7, double(score))) : undefined |
@isiahmeadows Yeah, makes sense. To clarify, I didn't mean that my "alternative examples" were 100% equivalent to the getScoreOrNull
|> ifNotNull(..)
|> ifNotNull(..) is something I could see myself using as a workaround instead of nested pipelines or breaking the pipeline to a statement, at least in some cases. But it's definitely less than ideal workaround, a short-circuiting |
@funkjunky I didn't write a proposal for this, but I would love to include it here: https://xixixao.github.io/proposal-hack-pipelines/index.html#sec-additional-feature-op . |
Should |
I believe it should evaluate to |
@phaux The idea is that it'd carry the same propagation semantics, so it would return |
The nuance of language creation is something I'd leave to people more experience than me. Null or undefined... it's not very important to me. Personally I do think returning what the last piped function returned would be more useful to a developer, but I find very few uses in differentiating between null and undefined. Just to be sure, I think we all agree that false should pass through and continue piping. @noppa You're example is awesome, but I think it'd help to add one more example to show an additional beenfit of the optional operator:
I may have gotten the syntax wrong, but you get the idea. If you wanted to make the last 3 functions only be piped if non-null, then it becomes very messy. |
Are you going to write it or do you want me to write it? I'd like to help in any way that can turn this addition closer to a reality :p [I feel like I'm must continue the war fighting with other devs to show javascript is a great powerful language... unfortunately in my 12 year career I've met far too many, a majority, [bad :p] programmers who disparage Javascript] |
No need to nest them: let newScore = person
|> getScoreOrNull
|> ifNotNull(double)
|> ifNotNull(_ => add(7, _))
|> ifNotNull(_ => boundScore(0, 100, _)) |
The un‑nested version won’t have the correct short‑circuiting behaviour. |
The idea was to show a benefit, so there should be a better option than unrealistic nested functions to avoid evaluating expressions with no side-effects. |
It won't? From what I can tell, it mostly would. The only difference is, it would call ifNotNull two additional times, while the nesting would not. Because the value doesn't change between these calls they're functionally equivilent. I think my biggest problem with Because the following (that @noppa wrote) are not logically equivilent:
If getScoreOrNull returns null,then they are equivilent, but if getScoreOrNull returns a non-null value and then double returns null, the first block would return null, until passed through all ifNotNull functions, the second block would call double with null, and continue to call each function if non-null values are returned. This leaves us with this dangerous piece of code:
One may think we'll stop at That's where the We need something like |
IMHO making this combination (optional+pipeline chaining) is an important one and feels more like applying the idea of "optional chaining" evenly across the language. People, being familiar with optional chaining already, once they start using the pipeline chaining, will definitely run into situations where they instinctively want to combine the two. |
With the current hack-style proposal, I guess what I'd do is something like const notNil = val => val == null ? undefined : true
let newScore = person
|> getScoreOrNull(^)
|> (notNil(^) && (double(^)
|> add(7, ^)
|> boundScore(0, 100, ^)
)) Maybe not the ugliest piece of code ever but it's not great either with all the extra parens and stuff. |
Yeah, I think (I mean, ultimately, this is just baking the Option monad into the language at the syntax level, just as |
That code style is hard to look at without wincing… |
Yeah it's not great. Personally, I'd probably just break the pipe and use an |
Why not a `fold` to keep the pipeline processing composable & functional?
|
Could you expand on that? |
@tabatkins Sure: You'd avoid breaking up the pipe for // Written for the sake of being consistent with how I explained `fold`
const doSomethingWithValidValue = value => value |> double(^) |> add(7, ^) |> boundScore(0. 100, ^)
const newScore = person
|> getScoreOrNull(^)
|> fold(doSomethingWithNullValue, doSomethingWithValidValue) Edited: Corrected typo of |
(Assuming Yeah, you can def abstract it away, but that does mean you can't write your pipe inline; you have to pull the later chunk out into a separate function. |
Correct, it was meant to be an arrow function. It's equivalent doing the following (assuming fold is data-last, but it doesn't have to be, just more aligning with the natural composition nature of it all pipeline operator typically helps illustrate): fold(doSomethingWithNullValue, doSomethingWithValidValue, getScoreOrNull(x)); |
@tabatkins Am I correct to realize that the following code snippet for the curryable |> fold(doSomethingWithNullValue, doSomethingWithValidValue)` Would have to be the following instead? |> fold(doSomethingWithNullValue, doSomethingWithValidValue)(^) If so…. I have to say that immediately adds cognitive load for a operator usually associated with being a functional composition operator that facilities functional chaining first & foremost (which I'm aware needs to be sold as the common case most perceive of it; I think that's very feasible to prove towards a possible syntax change during this stage of the proposal). Just to be clear, I'm only pointing out |
You simply wouldn't write a curried function fold(nullCB, nonNullCB) {
return value=>{
...actual function code here...
}
} If you're intending to use it non-HOF code, you'd just write function fold(value, nullCB, nonNullCB) {
...actual code here...
}
// or maybe with "value" as last arg,
// whatever floats your boat
// and then you can write
const x = val |> fold(^, cb1, cb2) |> ...
// or just this, if appropriate
const x = fold(val, cb1, cb2); If you want to be flexible to use it in both contexts, you're already in a minority of a minority, but it's not too hard to write a transformer function that can convert a function in one of the forms to allowing both forms. Ramda has an auto-currying transformer, I know; I expect most HOF-oriented utility libraries do. |
Edit: Fix minor semantic mistake
It's not an uncommon occurrence to want to do something like this:
If support for optional chaining was added, this could be simplified to just this, providing some pretty massive wins in source size (I'm using Elixir-style syntax here, but you could translate it to others easily enough):
Here's what they'd look minified, if you're curious
The text was updated successfully, but these errors were encountered: