Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

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

Closed
claudepache opened this issue Jul 24, 2017 · 38 comments
Closed

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

claudepache opened this issue Jul 24, 2017 · 38 comments

Comments

@claudepache
Copy link
Collaborator

claudepache commented Jul 24, 2017

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:

  • simple assignment: a?.b = c
  • combined assignment: a?.b += c, a?.b >>= c, etc.
  • increment/decrement: a?.b++, --a?.b, etc.
  • destructuring assignment: { x: a?.b } = c, [ a?.b ] = c, etc.
  • for-in/of loop header: for (a?.b in c), for (a?.b of c)
@alangpierce
Copy link

alangpierce commented Jul 24, 2017

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 #17 (comment), two of them seemed like they might be affected by this:

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

@claudepache
Copy link
Collaborator Author

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 claudepache changed the title Should we include “optional assignment” a?.b = c Should we include “optional property assignment” a?.b = c Jul 27, 2017
@claudepache
Copy link
Collaborator Author

claudepache commented Jul 27, 2017

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

  • simple assignment: a?.b = c
  • combined assignment: a?.b += c, a?.b >>= c, etc.
  • increment/decrement: a?.b++, --a?.b, etc.
  • destructuring assignment: { x: a?.b } = c, [ a?.b ] = c, etc.
  • for-in/of loop header: for (a?.b in c), for (a?.b of c)

@tc39 tc39 deleted a comment from ljharb Jul 27, 2017
@victornpb
Copy link

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
Copy link
Member

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

@claudepache
Copy link
Collaborator Author

@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
Copy link
Member

Is it this one, from #17 usage statistics?

Total soaked assignments (including compound assignments): 37

@claudepache
Copy link
Collaborator Author

Is it this one, from #17 usage statistics?

Yes.

@roippi
Copy link

roippi commented Sep 20, 2017

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
Copy link

samuelgoto commented Sep 22, 2017

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

claudepache commented Sep 25, 2017

@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
Copy link
Member

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

@littledan
Copy link
Member

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

claudepache added a commit that referenced this issue Nov 13, 2017
@claudepache
Copy link
Collaborator Author

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.)

@tc39 tc39 deleted a comment from Mouvedia Nov 13, 2017
@tc39 tc39 deleted a comment from ariporad Nov 13, 2017
@tc39 tc39 deleted a comment from jridgewell Nov 13, 2017
@tc39 tc39 deleted a comment from ariporad Nov 13, 2017
@tc39 tc39 deleted a comment from ariporad Nov 13, 2017
@tc39 tc39 deleted a comment from ariporad Nov 13, 2017
@tc39 tc39 deleted a comment from Nokel81 Nov 13, 2017
@tc39 tc39 deleted a comment from Nokel81 Nov 13, 2017
@tc39 tc39 deleted a comment from Nokel81 Nov 13, 2017
@dannyskoog
Copy link

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

@javan
Copy link

javan commented Aug 14, 2019

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
Copy link

Mouvedia commented Aug 14, 2019

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
Copy link
Collaborator Author

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
Copy link

Mouvedia commented Aug 14, 2019

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
Copy link
Member

ljharb commented Aug 14, 2019

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
Copy link

Mouvedia commented Aug 14, 2019

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
Copy link
Collaborator Author

@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
Copy link

dantman commented Aug 14, 2019

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.

  • a += b is equivalent to a = a + b (the value of b is added to a and then assigned to a)
  • JS does not have ||= yet, but it does exist in other languages. If JS did have it then a ||= b would likely be equivalent to a = a || b (if a is falsey then set a to b).
  • Logically expanding on that if we were able to justify ||= we would probably also add ??= for which a ??= b would be equivalent to a = a ?? b (if a is nullish then set a to b).
  • I suppose from that logic it would be wrong to intuit a ?= b as "if b is something" since the others are "if a is something".
  • However that still leaves the discrepancy that ?= does not make the logical jump to "?. in all places of a.b". Honestly I think someone looking at ?= would quicker jump to a ternary. e.g. a ?= b might be perceived as a = a ? a : b, but that would be the same as a ||= b and a ?:= b would probably make more sense.

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
Copy link

Mouvedia commented Aug 14, 2019

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

@javan
Copy link

javan commented Aug 14, 2019

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
Copy link
Member

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
Copy link
Member

hax commented Aug 19, 2019

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
Copy link

dantman commented Aug 21, 2019

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
Copy link
Member

hax commented Aug 22, 2019

both of these are invalid syntax.

@dantman
This issue is discussing whether we should support optional property assignment (in the future), see #18 (comment) .

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
Copy link

FrankFang commented Aug 22, 2019

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
Copy link
Member

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
Copy link
Collaborator Author

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 #18 (comment) 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
Copy link

@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
Copy link

avalanche1 commented Dec 4, 2021

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
Copy link

jazeee commented Dec 4, 2021

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests