tc39 / proposal-optional-chaining

https://tc39.github.io/proposal-optional-chaining/
4.94k stars 75 forks source link

Should we include “optional property assignment” a?.b = c #18

Closed claudepache closed 6 years ago

claudepache commented 7 years ago

For the most general case illustrating short-circuiting, the instruction:

a?.b.c().d = e;

is equivalent to:

if (a != null)
    a.b.c().d = e;

@alangpierce has found real-world usages of that form in CoffeeScript, some of them seem reasonable, including two-level deep chains a?.b.c = d: https://gist.github.com/alangpierce/34b7aa40cda51b0a089a44680bdfed7e

Should we include that? In particular, are the semantics clear enough? There are the following subcases:

alangpierce commented 7 years ago

Another question is whether this code should evaluate c() if a is null:

a?.b = c()

In CoffeeScript, it skips the evaluation of c() if a is null. In other words, the short-circuiting extends all the way up to assignment operations. But an alternate interpretation is that a?.b evaluates to a writable reference that does nothing when you assign to it, meaning that c() still gets evaluated.

Skipping the evaluation of c() seems particularly "magical" in my opinion and I'd prefer to always evaluate c().

Of the 37 examples from https://github.com/tc39/proposal-optional-chaining/issues/17#issuecomment-317321279, two of them seemed like they might be affected by this:

@inputElement?.value = Trix.serializeToContentType(this, "text/html")
@activeHintMarker?.style?.zIndex = getNextZIndex()
claudepache commented 7 years ago

Unless you rely on side-effects – in which case you are responsible to learn exactly if and when evaluation occurs –, you won't see a difference. So, in order to avoid unnecessary work from the engine, I am for skipping the evaluation of the RHS.

claudepache commented 7 years ago

AFAICT, there are five cases where property assignment may occur, and where optional property assignment might be considered:

victornpb commented 7 years ago

Another question is whether this code should evaluate c() if a is null:

a?.b = c()

if

a?.b.c().d = e;

is equivalent to:

if (a != null)
    a.b.c().d = e;

then

a?.b = c()

is quivalent to:

if(a != null) a.b = c();
littledan commented 7 years ago

This all gets a bit complicated. Do you have concrete use scenarios in mind that justify the complexity?

claudepache commented 7 years ago

@littledan Basically, concrete use cases are from the CoffeeScript samples linked in Comment 0. I’ve not delved deep into them, but here are two excerpts that seem reasonable at first sight:

@component?.element.component = null

and:

    modal = $('#create-account-modal').data('bs.modal')
    modal?.options?.keyboard = false
littledan commented 7 years ago

Is it this one, from #17 usage statistics?

Total soaked assignments (including compound assignments): 37

claudepache commented 7 years ago

Is it this one, from #17 usage statistics?

Yes.

roippi commented 7 years ago

My experience from dealing with assigning to deeply-nested-possibly-null structures has almost universally been that when there's a null I want to actually create the key and then successfully assign to it. I'm hard pressed to come up with a time when I'd want to skip the assignment entirely.

samuelgoto commented 7 years ago

Just wondering on the original question: if you leave assignment out, does it corner you? That is, if you completely ignore this feature and get the "read" in, does it prevent you (or make it harder) from ever doing the "write/assignment"?

If it doesn't, then it might be smart to de-couple and discuss these features in isolation (the "read" and the "write" separately) and turn this into a sequencing problem (i.e. in which order we bake things into the language, rather if we should or not)?

claudepache commented 7 years ago

@roippi

My experience from dealing with assigning to deeply-nested-possibly-null structures has almost universally been that when there's a null I want to actually create the key and then successfully assign to it. I'm hard pressed to come up with a time when I'd want to skip the assignment entirely.

Deeply-nested-possibly-null structures are not the unique use-case of optional chaining.

Here is a synthetic example. Suppose that I have an element of the DOM. If the element is found inside a <details> section, I want to open that section:

elem.closest('details')?.open = true
claudepache commented 7 years ago

@samuelgoto There is no technical problem in postponing the “write” case. If that helps to accept the “read” case more rapidly, it will be indeed postponed.

littledan commented 7 years ago

OK, seems like there's a rough agreement in this thread not to do the write case in the first iteration.

littledan commented 6 years ago

We also discussed this question at TC39, and the committee did not seem so interested in adding this feature.

claudepache commented 6 years ago

Somewhat sad, but ok. I've just updated the README, stating that it won’t be supported for now.

I'm also going to delete the quite numerous old off-topic comments in this thread, in order to keep this discussion more readable for future reference. (Anyway, thanks to all for the feedback, even for off-topic one.)

Favitalegur commented 5 years ago

Hack alert: a?.b[a.b = c ];

DanPen commented 5 years ago

I'd love to have this feature.

dannyskoog commented 5 years ago

To me the ”read” case is by far the most usable between the two. The ”write” case would be a nice future bonus

javan commented 5 years ago

Mark me down as another I'd love to have this feature if such a tally exists. I'm one of the authors of Trix, and am happy to see its usage included in #17 (thanks, @alangpierce!).

I don't write much CoffeeScript these days, but do use optional chaining via Babel's plugin. Lack of support for assignment caught me by surprise right off the bat. Here's a snippet of code I attempted to write (extracted from a class body):

  append({ id, html }) {
    const element = document.getElementById(id)
    element?.insertAdjacentHTML("beforeend", html)
  }

  prepend({ id, html }) {
    const element = document.getElementById(id)
    element?.insertAdjacentHTML("afterbegin", html)
  }

  replace({ id, html }) {
    const element = document.getElementById(id)
    element?.outerHTML = html
  }

  update({ id, html }) {
    const element = document.getElementById(id)
    element?.innerHTML = html
  }

Nice and symmetrical, right? Unfortunately, the last two methods are invalid, and require an additional element conditional around the assignment.

Mouvedia commented 5 years ago

Concerning optional/conditional assignment, Id prefer something like

element.innerHTML ?= html;

You don't need to be granular about it, i.e. any link/node/property in the chain could be missing the result would be the same: no assignment.

claudepache commented 5 years ago

You don't need to be granular about it

I do want to be granular in absence of good reason for the contrary. It’s way better both for debugging and for understanding the meaning of code you didn’t just write.

Mouvedia commented 5 years ago

It’s way better both for debugging and for understanding the meaning of code you didn’t just write.

If I wanted to be explicit about it I wouldn't have used a shorthand and just relied on an if block. Being granular is not the goal here. What matters is concision and the assignment, not which property was the culprit. You don't want to "debug" because you are expecting the failure.

It would be a perversion to constantly use ?= as a form of defensive programming.

ljharb commented 5 years ago

Conciseness without granularity is something I’d personally consider an antipattern anywhere.

The goal of the proposal is, for a specific value only, to avoid operating on it if it’s nullish. That does indeed require being granular.

Mouvedia commented 5 years ago

The goal of the proposal is…

What I am saying is that I consider this usage a separate matter that doesn't require the granularity. ?= was a bad choice on my part though because of the absence of an expanded variant that would mimick existing +=/-= and upcoming ||=/&&= counterparts.

claudepache commented 5 years ago

@Mouvedia From experience (not restricted to programming), I can say it is very advantageous to be both concise and precise.

For this particular example (element?.innerHTML = html), they want to say exactly: “if element is null/undefined, skip the assignment”; even if the precision does not appear paramount at the very moment they’re writing the code.

dantman commented 5 years ago

Concerning optional/conditional assignment, Id prefer something like

element.innerHTML ?= html;

Honestly, to me element.innerHTML ?= html; does not read as "set element.innerHTML to html if element is not nullish". To me it reads more like "set element.innerHTML to html if html is not nullish.

That's how I personally intuit that; but let's see how the logic holds up.

IMHO ?= has too many possible interpretations to ever add to the language to mean one specific thing.

Also if we use a?.b = c instead of a.b ?= c it means that if we add ??= it's possible to write a?.b ??= c (if (a != null) a.b = a.b != null ? a.b : c;) instead of optional chaining assignment and nullish assignment being mutually exclusive.

Mouvedia commented 5 years ago

If ?? reaches stage 4 we will eventually have ??=. I wish they would add all the <operator>= as a batch though.

javan commented 5 years ago

In case it's not clear in my comment above, I'm not suggesting an alternate syntax like element.innerHTML ?= html or expressing any concern with the syntax in this proposal.

I would just like to see optional property assignment supported (e.g. element?.innerHTML = html).

jridgewell commented 5 years ago

If ?? reaches stage 4 we will eventually have ??=. I wish they would add them all the <operator>= as a batch though.

They'll all be added at the same time. https://github.com/tc39/proposal-logical-assignment

hax commented 5 years ago
for (a?.b of [1,2,3]) {...}
({x: a?.b} = {});
a?.b ??= d;

All these code looks confusing (got new killer questions for js interview 🤯)... I hope we could disallow them if we add optional assignment 😂

dantman commented 5 years ago
for (a?.b of [1,2,3]) {...}
({x: a?.b} = {});

Optional chains are false for IsDestructuring and IsSimpleAssignmentTarget so both of these are invalid syntax.

a?.b ??= d;

This makes perfect logical sense (if (a != null && a.b != null) a.b = d;), so I see no reason to overcomplicate the spec by disallowing it if we added optional assignment.

hax commented 5 years ago

both of these are invalid syntax.

@dantman This issue is discussing whether we should support optional property assignment (in the future), see https://github.com/tc39/proposal-optional-chaining/issues/18#issue-244992254 .

This makes perfect logical sense if (a != null && a.b != null) a.b = d;

a?.b ??= d;

Some programmers roughly read it as if (a?.b == null) a.b = d; and expect it work as

if (a?.b == null) {
  if (a == null) a = {}
  a.b = d;
}
FrankFang commented 5 years ago

It's too complicated.

if (a != null)
    a.b.c().d = e;

could be

tmp = a?.b?.c()
if(tmp != null){
  tmp.d = e
}
jridgewell commented 5 years ago

Note, a ??= b is a different proposal (translating to a ?? (a = b).

Some programmers roughly read it as if (a?.b == null) a.b = d; and expect it work as

I also think this is the wrong way to do it. Creating the intermediate objects seems like magic.

claudepache commented 5 years ago

Note, a ??= b is a different proposal

I was going say it more energetically: Please, stop discussing a ??= b in this thread: it is a different operator that does a different thing and solve different problems. Don’t be confused by https://github.com/tc39/proposal-optional-chaining/issues/18#issuecomment-521352449 above, that proposed something completely unrelated to ??=.

@dantman

a?.b ??= d;

This makes perfect logical sense

While it would have a well-defined theoretical meaning (although not exactly the one you gave), it is basically taking two unrelated operators and mixing them randomly in a same expression. The fate of such an expression could very well be the same as [a,b] += d, i.e. be statically forbidden. But such fringe edge cases should only be discussed in a dedicated thread in the possible forthcoming proposal about a?.b = c.

clouds56 commented 4 years ago

@Favitalegur

Hack alert: a?.b[a.b = c ];

should this be a?.[a.b=c];? for the case a.b is undefined. most precise a?.[a.b=c,"b"]

avalanche1 commented 2 years ago

We also discussed this question at TC39, and the committee did not seem so interested in adding this feature.

Thanks guys! Now instead of writing this document.querySelector(".bulk-send-progress")?.style.display = "none"; Ima have to write this

    if (document.querySelector(".bulk-send-progress")) {
      document.querySelector(".bulk-send-progress").style.display = "none";
    }
jazeee commented 2 years ago

First off, I much appreciate the effort that went in to optional chaining and null coalescing functionality. A huge benefit for the community.

We also discussed this question at TC39, and the committee did not seem so interested in adding this feature.

Thanks guys! Now instead of writing this document.querySelector(".bulk-send-progress")?.style.display = "none"; Ima have to write this

    if (document.querySelector(".bulk-send-progress")) {
      document.querySelector(".bulk-send-progress").style.display = "none";
    }

In this example, it really sounds like you would like to propose a new feature. This is well within your power to do so. It makes a ton of sense for the original proposal to limit scope, while allowing for further improvements over time, such as your suggestion.

Also, I suppose I should add, if you are calling a function twice like this, you would be better off doing something like:

  const element = document.querySelector(".bulk-send-progress");
  if (element) {
    element.style.display = "none";
  }

While it does not address your initial request, it at least limits the function call to a single execution, and also simplifies the code, reducing copy paste errors and the like. Indeed, even if there were a feature for optional property assignment, I'd likely still write it as:

  const element = document.querySelector(".bulk-send-progress");
  element?.style.display = "none";

This would improve code clarity, and reduce overly long lines.

In any case, I highly encourage you to introduce a proposal via the TC39 processes, and carry it through to completion. https://github.com/tc39/ecma262/blob/HEAD/CONTRIBUTING.md

Hope this helps in getting new features into the EcmaScript standards. I look forward to seeing your contributions, since this feature could be of value.